-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
converter for istio in-cluster operator config to sail operator config #616
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,193 @@ | ||
// Copyright Istio Authors | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package converter | ||
|
||
import ( | ||
"fmt" | ||
"os" | ||
"path/filepath" | ||
"strings" | ||
"testing" | ||
|
||
"github.com/istio-ecosystem/sail-operator/pkg/test/project" | ||
"github.com/istio-ecosystem/sail-operator/pkg/test/util/supportedversion" | ||
"github.com/istio-ecosystem/sail-operator/tests/e2e/util/shell" | ||
. "github.com/onsi/gomega" | ||
) | ||
|
||
var ( | ||
controlPlaneNamespace = "istio-system" | ||
istioVersion = supportedversion.Default | ||
istioFile = filepath.Join(project.RootDir, "tools", "istioConfig.yaml") | ||
sailFile = filepath.Join(project.RootDir, "tools", "istioConfig-sail.yaml") | ||
) | ||
|
||
func TestConversion(t *testing.T) { | ||
t.Cleanup(func() { | ||
os.Remove(istioFile) | ||
os.Remove(sailFile) | ||
}) | ||
testcases := []struct { | ||
name string | ||
input string | ||
expectedOutput string | ||
}{ | ||
{ | ||
name: "simple", | ||
input: `apiVersion: install.istio.io/v1alpha1 | ||
kind: IstioOperator | ||
metadata: | ||
name: default | ||
spec:`, | ||
expectedOutput: `apiVersion: sailoperator.io/v1 | ||
kind: Istio | ||
metadata: | ||
name: default | ||
spec: | ||
version: %s | ||
namespace: %s`, | ||
}, | ||
{ | ||
name: "complex", | ||
input: `apiVersion: install.istio.io/v1alpha1 | ||
kind: IstioOperator | ||
metadata: | ||
name: default | ||
spec: | ||
components: | ||
base: | ||
enabled: true | ||
pilot: | ||
enabled: false | ||
values: | ||
global: | ||
externalIstiod: true | ||
operatorManageWebhooks: true | ||
configValidation: false | ||
base: | ||
enableCRDTemplates: true | ||
pilot: | ||
env: | ||
PILOT_ENABLE_STATUS: true`, | ||
expectedOutput: `apiVersion: sailoperator.io/v1 | ||
kind: Istio | ||
metadata: | ||
name: default | ||
spec: | ||
values: | ||
global: | ||
externalIstiod: true | ||
operatorManageWebhooks: true | ||
configValidation: false | ||
base: | ||
enableCRDTemplates: true | ||
enabled: true | ||
pilot: | ||
env: | ||
PILOT_ENABLE_STATUS: "true" | ||
enabled: false | ||
version: %s | ||
namespace: %s`, | ||
}, | ||
{ | ||
name: "mandatory-arguments-only", | ||
input: `apiVersion: install.istio.io/v1alpha1 | ||
kind: IstioOperator | ||
metadata: | ||
name: default | ||
spec:`, | ||
expectedOutput: `apiVersion: sailoperator.io/v1 | ||
kind: Istio | ||
metadata: | ||
name: default | ||
spec: | ||
version: %s | ||
namespace: %s`, | ||
}, | ||
} | ||
for _, tc := range testcases { | ||
t.Run(tc.name, func(t *testing.T) { | ||
g := NewWithT(t) | ||
err := saveYamlToFile(tc.input, istioFile) | ||
g.Expect(err).To(Succeed(), "failed to write YAML file") | ||
|
||
if tc.name == "mandatory-arguments-only" { | ||
g.Expect(executeConfigConverter(istioFile, "", "", istioVersion)).To(Succeed(), "error in execution of ./configuration-converter.sh") | ||
} else { | ||
g.Expect(executeConfigConverter(istioFile, sailFile, controlPlaneNamespace, | ||
istioVersion)).To(Succeed(), "error in execution of ./configuration-converter.sh") | ||
} | ||
tc.expectedOutput = fmt.Sprintf(tc.expectedOutput, istioVersion, controlPlaneNamespace) | ||
isConversionValidated, err := validateYamlContent(tc.expectedOutput, sailFile) | ||
g.Expect(err).To(Succeed(), "Can not open file to compare") | ||
g.Expect(isConversionValidated).To(BeTrue(), "Converted content is not as expected") | ||
}) | ||
} | ||
} | ||
|
||
// saveYamlToFile writes the given YAML content to given file | ||
func saveYamlToFile(input, istioFile string) error { | ||
if err := os.WriteFile(istioFile, []byte(input), 0o644); err != nil { | ||
return fmt.Errorf("failed to write YAML file: %w", err) | ||
} | ||
return nil | ||
} | ||
|
||
func executeConfigConverter(input, output, controlPlaneNamespace, istioVersion string) error { | ||
converter := filepath.Join(project.RootDir, "tools", "configuration-converter.sh") | ||
args := []string{ | ||
converter, | ||
"-i", input, | ||
"-v", istioVersion, | ||
} | ||
|
||
if output != "" { | ||
args = append(args, "-o", output) | ||
} | ||
|
||
if controlPlaneNamespace != "" { | ||
args = append(args, "-n", controlPlaneNamespace) | ||
} | ||
|
||
cmd := strings.Join(args, " ") | ||
_, err := shell.ExecuteCommand(cmd) | ||
if err != nil { | ||
return fmt.Errorf("error in execution of %s -i %s -o %s -n %s -v %s: %w", converter, input, output, controlPlaneNamespace, istioVersion, err) | ||
} | ||
return nil | ||
} | ||
|
||
// compareYamlContent checks if the provided YAML content matches the content of converted sail operator config | ||
func validateYamlContent(expectedOutput, sailFile string) (bool, error) { | ||
// Write the input YAML string to a temporary file | ||
tmpFile := filepath.Join(project.RootDir, "tools", "temp.yaml") | ||
err := os.WriteFile(tmpFile, []byte(expectedOutput), 0o644) | ||
if err != nil { | ||
return false, fmt.Errorf("failed to write temporary YAML file: %w", err) | ||
} | ||
defer os.Remove(tmpFile) | ||
|
||
// The command will check if the files are equal, ignoring order | ||
cmd := fmt.Sprintf("diff <(yq -P 'sort_keys(..)' -o=props %s) <(yq -P 'sort_keys(..)' -o=props %s)", sailFile, tmpFile) | ||
output, err := shell.ExecuteCommand(cmd) | ||
if err != nil { | ||
return false, fmt.Errorf("error executing yq comparison: %w", err) | ||
} | ||
if output != "" { | ||
return false, nil | ||
} | ||
// If no output from yq, the files are considered equal | ||
return true, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
#!/bin/bash | ||
|
||
# Copyright Istio Authors | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
# This script is used to convert istio configuration to sail operator configuration. | ||
# In the end of the execution new yaml file will be created with "sail-ISTIO_CONFIG_YAML" name. | ||
# Usage: ./configuration-converter.sh ISTIO_CONFIG_YAML_WITH_PATH, example: ./configuration-converter.sh sample_config.yaml" | ||
set -e | ||
# Default values | ||
NAMESPACE="istio-system" | ||
VERSION="" | ||
INPUT="" | ||
OUTPUT="" | ||
|
||
# Function to show usage | ||
usage() { | ||
echo "Usage: $0 -i <input> [-o <output>] [-n <namespace>] [-v <version>]" | ||
echo " -i <input> : Input file (required)" | ||
echo " -o <output> : Output file (optional, defaults to input's base directory with '-sail.yaml' suffix)" | ||
echo " -n <namespace> : Namespace (optional, defaults to 'istio-system')" | ||
echo " -v <version> : Version (optional)" | ||
exit 1 | ||
} | ||
|
||
# Parse command-line arguments | ||
while [[ $# -gt 0 ]]; do | ||
case "$1" in | ||
-i) | ||
INPUT="$2" | ||
shift 2 | ||
;; | ||
-o) | ||
OUTPUT="$2" | ||
shift 2 | ||
;; | ||
-n) | ||
NAMESPACE="$2" | ||
shift 2 | ||
;; | ||
-v) | ||
VERSION="$2" | ||
shift 2 | ||
;; | ||
*) | ||
usage | ||
;; | ||
esac | ||
done | ||
|
||
# Ensure input file is provided | ||
if [[ -z "$INPUT" ]]; then | ||
echo "Error: Input file is required." | ||
usage | ||
fi | ||
|
||
ctartici marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# Set default output file if not provided. | ||
if [[ -z "$OUTPUT" ]]; then | ||
OUTPUT="$(dirname "$INPUT")/$(basename "$INPUT" .yaml)-sail.yaml" | ||
elif [[ -d "$OUTPUT" ]]; then | ||
echo "Error: OUTPUT must be a file, not a directory." | ||
exit 1 | ||
fi | ||
|
||
if ! command -v yq &>/dev/null; then | ||
echo "Error: 'yq' is not installed. Please install it before running the script." | ||
exit 1 | ||
fi | ||
|
||
function add_mandatory_fields(){ | ||
yq -i eval ".apiVersion = \"sailoperator.io/v1\" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would add a check for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It will fail with yq command not found. You think I need to catch command not found and inform user with better error message? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could add the following function.
Or just check and explicitly tell the user what is missing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. added an error message for yq installation |
||
| .kind = \"Istio\" | ||
| (select(.spec.meshConfig) | .spec.values.meshConfig) = .spec.meshConfig | ||
| (select(.spec.values.istio_cni) | .spec.values.pilot.cni) = .spec.values.istio_cni | ||
| .metadata.name = \"default\" | ||
| .spec.version = \"$VERSION\" | ||
| .spec.namespace = \"$NAMESPACE\" | ||
| del(.spec.values.istio_cni) | ||
| del(.spec.meshConfig) | ||
| del(.spec.hub) | ||
| del(.spec.tag) | ||
| del(.spec.values.gateways)" "$OUTPUT" | ||
} | ||
|
||
#Convert boolean values to string if they are under *.env | ||
function boolean_2_string(){ | ||
yq -i -e '(.spec.values.[].env.[] | select(. == true)) |= "true" | ||
| (.spec.values.[].env.[] | select(. == false)) |= "false"' "$OUTPUT" | ||
yq -i -e 'del(.. | select(tag == "!!seq" and length == 0))' "$OUTPUT" | ||
} | ||
|
||
# Note that if there is an entry except spec.components.<component>.enabled: true/false converter will delete them and warn user | ||
function validate_spec_components(){ | ||
yq -i 'del(.spec.components.[] | keys[] | select(. != "enabled")) | .spec.values *= .spec.components | del (.spec.components)' "$OUTPUT" | ||
echo "Values only spec.components.<component>.enabled: true/false are supported for conversion. Please see https://github.com/istio-ecosystem/sail-operator/tree/main/docs#components-field" | ||
} | ||
|
||
# create output file | ||
cp "$INPUT" "$OUTPUT" | ||
|
||
# in-place edit created output file | ||
add_mandatory_fields | ||
boolean_2_string | ||
if [[ $(yq eval '.spec.components' "$OUTPUT") != "null" ]]; then | ||
validate_spec_components | ||
fi | ||
|
||
echo "Sail configuration file created with name: $(realpath "$OUTPUT")" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to have to specify
-i
instead of just the file name? You always need to specify the input file, so why make the user type-i
? Similar for-o
, even though that one is optional. The input and output seem like the most important arguments, so we don't need to specify them via flags. The namespace and version arguments are not at the same level (much more optional than the output file), so it makes sense to set them via flags.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I have removed -i and -o arguments and updated readme