From 5f4249550042a6af99804ffabeb2edb8db48ff08 Mon Sep 17 00:00:00 2001 From: smcavallo Date: Mon, 28 Aug 2023 22:23:12 -0400 Subject: [PATCH] fix(eks): Add reconciliation for addon configurationValues Signed-off-by: smcavallo --- examples/eks/addon.yaml | 12 +- pkg/controller/eks/addon/setup.go | 49 ++++++ pkg/controller/eks/addon/setup_test.go | 222 +++++++++++++++++++++++++ 3 files changed, 282 insertions(+), 1 deletion(-) diff --git a/examples/eks/addon.yaml b/examples/eks/addon.yaml index 0a6cdedcf9..95fa206b0d 100644 --- a/examples/eks/addon.yaml +++ b/examples/eks/addon.yaml @@ -9,7 +9,17 @@ spec: forProvider: region: us-east-1 addonName: coredns - addonVersion: "v1.8.4-eksbuild.1" + addonVersion: v1.10.1-eksbuild.2 + configurationValues: | + affinity: + nodeAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + nodeSelectorTerms: + - matchExpressions: + - key: my-nodegroup + operator: In + values: + - my-nodegroup-name clusterNameRef: name: sample-cluster providerConfigRef: diff --git a/pkg/controller/eks/addon/setup.go b/pkg/controller/eks/addon/setup.go index e9072d86f6..b061871ebf 100644 --- a/pkg/controller/eks/addon/setup.go +++ b/pkg/controller/eks/addon/setup.go @@ -19,11 +19,14 @@ package addon import ( "context" + "github.com/aws/aws-sdk-go/aws" awseks "github.com/aws/aws-sdk-go/service/eks" "github.com/aws/aws-sdk-go/service/eks/eksiface" + "github.com/google/go-cmp/cmp" "github.com/pkg/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/yaml" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/connection" @@ -144,10 +147,56 @@ func (h *hooks) isUpToDate(_ context.Context, cr *eksv1alpha1.Addon, resp *awsek return false, "", nil } + // AddOn Configuration Values + configUpToDate, configUpToDateDiff, configUpToDateErr := isUpToDateConfigurationValues(cr, resp) + if configUpToDateErr != nil { + return false, "", configUpToDateErr + } + if !configUpToDate { + return false, configUpToDateDiff, nil + } + add, remove := awsclients.DiffTagsMapPtr(cr.Spec.ForProvider.Tags, resp.Addon.Tags) return len(add) == 0 && len(remove) == 0, "", nil } +func isUpToDateConfigurationValues(cr *eksv1alpha1.Addon, obj *awseks.DescribeAddonOutput) (bool, string, error) { + // Handle nil pointer refs + crConfigurationValues := aws.StringValue(cr.Spec.ForProvider.ConfigurationValues) + objConfigurationValues := aws.StringValue(obj.Addon.ConfigurationValues) + if crConfigurationValues == "" && objConfigurationValues == "" { + return true, "", nil + } + + if crConfigurationValues == "" && objConfigurationValues != "" { + return false, "", nil + } + if crConfigurationValues != "" && objConfigurationValues == "" { + return false, "", nil + } + + // Normalize the data + objConfigurationValuesCmp, objErr := convertConfigurationValuesToObj(objConfigurationValues) + if objErr != nil { + return false, "", objErr + } + crConfigurationValuesCmp, crErr := convertConfigurationValuesToObj(crConfigurationValues) + if crErr != nil { + return false, "", crErr + } + // Compare objects and return a diff + diff := cmp.Diff(objConfigurationValuesCmp, crConfigurationValuesCmp) + return diff == "", diff, nil +} + +// convertConfigurationValuesToObj will deserialize in order to normalize and compare +func convertConfigurationValuesToObj(configurationValues string) (map[string]interface{}, error) { + var objConfigurationValues map[string]interface{} + // Yaml parser is able to handle Unmarshal for both YAML and JSON + err := yaml.Unmarshal([]byte(configurationValues), &objConfigurationValues) + return objConfigurationValues, err +} + func preUpdate(_ context.Context, cr *eksv1alpha1.Addon, obj *awseks.UpdateAddonInput) error { obj.ClusterName = cr.Spec.ForProvider.ClusterName return nil diff --git a/pkg/controller/eks/addon/setup_test.go b/pkg/controller/eks/addon/setup_test.go index 3740764253..3659468554 100644 --- a/pkg/controller/eks/addon/setup_test.go +++ b/pkg/controller/eks/addon/setup_test.go @@ -24,6 +24,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/pkg/errors" + "github.com/aws/aws-sdk-go/aws" awseks "github.com/aws/aws-sdk-go/service/eks" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" @@ -835,3 +836,224 @@ func TestDelete(t *testing.T) { }) } } + +func TestIsUpToDateConfigurationValues(t *testing.T) { + type args struct { + cr *v1alpha1.Addon + obj *awseks.DescribeAddonOutput + } + type want struct { + isUpToDate bool + isError bool + } + + cases := map[string]struct { + args + want + }{ + "UpToDateForEmpty": { + args: args{ + cr: &v1alpha1.Addon{ + Spec: v1alpha1.AddonSpec{ + ForProvider: v1alpha1.AddonParameters{ + ConfigurationValues: aws.String(""), + }, + }, + }, + obj: &awseks.DescribeAddonOutput{Addon: &awseks.Addon{ConfigurationValues: aws.String("")}}, + }, + want: want{ + isUpToDate: true, + isError: false, + }, + }, + "UpToDateForJson": { + args: args{ + cr: &v1alpha1.Addon{ + Spec: v1alpha1.AddonSpec{ + ForProvider: v1alpha1.AddonParameters{ + ConfigurationValues: aws.String("{\"controller\": {\"podAnnotations\": {\"fluentbit.io/exclude\": \"true\"}}}"), + }, + }, + }, + obj: &awseks.DescribeAddonOutput{Addon: &awseks.Addon{ConfigurationValues: aws.String("{\"controller\": {\"podAnnotations\": {\"fluentbit.io/exclude\": \"true\"}}}")}}, + }, + want: want{ + isUpToDate: true, + isError: false, + }, + }, + "UpToDateForJsonWithIndentationAndWhitespace": { + args: args{ + cr: &v1alpha1.Addon{ + Spec: v1alpha1.AddonSpec{ + ForProvider: v1alpha1.AddonParameters{ + ConfigurationValues: aws.String("{\"controller\": {\"podAnnotations\": {\"fluentbit.io/exclude\": \"true\"}}}"), + }, + }, + }, + obj: &awseks.DescribeAddonOutput{Addon: &awseks.Addon{ConfigurationValues: aws.String("{\"controller\": {\"podAnnotations\":\n{\"fluentbit.io/exclude\": \"true\" }}}\n")}}, + }, + want: want{ + isUpToDate: true, + isError: false, + }, + }, + "UpToDateForYaml": { + args: args{ + cr: &v1alpha1.Addon{ + Spec: v1alpha1.AddonSpec{ + ForProvider: v1alpha1.AddonParameters{ + ConfigurationValues: aws.String("controller:\\r\\n podAnnotations:\\r\\n fluentbit.io/exclude: \\\"true\\"), + }, + }, + }, + obj: &awseks.DescribeAddonOutput{Addon: &awseks.Addon{ConfigurationValues: aws.String("controller:\\r\\n podAnnotations:\\r\\n fluentbit.io/exclude: \\\"true\\")}}, + }, + want: want{ + isUpToDate: true, + isError: false, + }, + }, + "NotUpToDateForYamlToJson": { + args: args{ + cr: &v1alpha1.Addon{ + Spec: v1alpha1.AddonSpec{ + ForProvider: v1alpha1.AddonParameters{ + ConfigurationValues: aws.String("controller:\\r\\n podAnnotations:\\r\\n fluentbit.io/exclude: \\\"true\\"), + }, + }, + }, + obj: &awseks.DescribeAddonOutput{Addon: &awseks.Addon{ConfigurationValues: aws.String("{\"controller\": {\"podAnnotations\": {\"fluentbit.io/exclude\": \"true\"}}}")}}, + }, + want: want{ + isUpToDate: false, + isError: false, + }, + }, + "NotUpToDateForJsonToYaml": { + args: args{ + cr: &v1alpha1.Addon{ + Spec: v1alpha1.AddonSpec{ + ForProvider: v1alpha1.AddonParameters{ + ConfigurationValues: aws.String("controller:\\r\\n podAnnotations:\\r\\n fluentbit.io/exclude: \\\"true\\"), + }, + }, + }, + obj: &awseks.DescribeAddonOutput{Addon: &awseks.Addon{ConfigurationValues: aws.String("{\"controller\": {\"podAnnotations\": {\"fluentbit.io/exclude\": \"true\"}}}")}}, + }, + want: want{ + isUpToDate: false, + isError: false, + }, + }, + "NotUpToDateForEmptyCr": { + args: args{ + cr: &v1alpha1.Addon{ + Spec: v1alpha1.AddonSpec{ + ForProvider: v1alpha1.AddonParameters{}, + }, + }, + obj: &awseks.DescribeAddonOutput{Addon: &awseks.Addon{ConfigurationValues: aws.String("{}")}}, + }, + want: want{ + isUpToDate: false, + isError: false, + }, + }, + "NotUpToDateForEmptyObj": { + args: args{ + cr: &v1alpha1.Addon{ + Spec: v1alpha1.AddonSpec{ + ForProvider: v1alpha1.AddonParameters{ + ConfigurationValues: aws.String("{}"), + }, + }, + }, + obj: &awseks.DescribeAddonOutput{Addon: &awseks.Addon{}}, + }, + want: want{ + isUpToDate: false, + isError: false, + }, + }, + "NotUpToDateHandleInvalidJson": { + args: args{ + cr: &v1alpha1.Addon{ + Spec: v1alpha1.AddonSpec{ + ForProvider: v1alpha1.AddonParameters{ + ConfigurationValues: aws.String("{InvalidJson {}!}"), + }, + }, + }, + obj: &awseks.DescribeAddonOutput{Addon: &awseks.Addon{ConfigurationValues: aws.String("{}")}}, + }, + want: want{ + isUpToDate: false, + isError: true, + }, + }, + "NotUpToDateHandleCrGarbage": { + args: args{ + cr: &v1alpha1.Addon{ + Spec: v1alpha1.AddonSpec{ + ForProvider: v1alpha1.AddonParameters{ + ConfigurationValues: aws.String("abcd1234"), + }, + }, + }, + obj: &awseks.DescribeAddonOutput{Addon: &awseks.Addon{ConfigurationValues: aws.String("{}")}}, + }, + want: want{ + isUpToDate: false, + isError: true, + }, + }, + "NotUpToDateForDifferentJson": { + args: args{ + cr: &v1alpha1.Addon{ + Spec: v1alpha1.AddonSpec{ + ForProvider: v1alpha1.AddonParameters{ + ConfigurationValues: aws.String("{\"controller\": {\"podAnnotations\": {\"fluentbit.io/exclude\": \"true\"}}}"), + }, + }, + }, + obj: &awseks.DescribeAddonOutput{Addon: &awseks.Addon{ConfigurationValues: aws.String("{\"controller\": {\"podAnnotations\": {\"fluentbit.io/exclude\": \"false\"}}}")}}, + }, + want: want{ + isUpToDate: false, + isError: false, + }, + }, + "NotUpToDateForDifferentYaml": { + args: args{ + cr: &v1alpha1.Addon{ + Spec: v1alpha1.AddonSpec{ + ForProvider: v1alpha1.AddonParameters{ + ConfigurationValues: aws.String("controller:\\r\\n podAnnotations:\\r\\n fluentbit.io/exclude: \\\"true\\"), + }, + }, + }, + obj: &awseks.DescribeAddonOutput{Addon: &awseks.Addon{ConfigurationValues: aws.String("controller:\\r\\n podAnnotations:\\r\\n fluentbit.io/exclude: \\\"false\\")}}, + }, + want: want{ + isUpToDate: false, + isError: false, + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + actualUpToDate, _, err := isUpToDateConfigurationValues(tc.args.cr, tc.args.obj) + + // Assert + if diff := cmp.Diff(tc.want.isUpToDate, actualUpToDate); diff != "" { + t.Errorf("r: -want, +got:\n%s", diff) + } + // Assert + if diff := cmp.Diff(tc.want.isError, err != nil); diff != "" { + t.Errorf("isError: -want, +got:\n%s", diff) + } + }) + } +}