Merge pull request #16937 from justinsb/sqs_policies_normalize

Fix: Normalize the SQS policies before comparing them

pkg/jsonutils/transform.go

@@ -0,0 +1,165 @@
+/*
+Copyright 2024 The Kubernetes 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 jsonutils
+
+import (
+	"encoding/json"
+	"fmt"
+	"sort"
+)
+
+// Transformer is used to transform JSON values
+type Transformer struct {
+	stringTransforms []func(path string, value string) (string, error)
+	objectTransforms []func(path string, value map[string]any) error
+	sliceTransforms  []func(path string, value []any) ([]any, error)
+}
+
+// NewTransformer is the constructor for a Transformer
+func NewTransformer() *Transformer {
+	return &Transformer{}
+}
+
+// AddStringTransform adds a function that will be called for each string value in the JSON tree
+func (t *Transformer) AddStringTransform(fn func(path string, value string) (string, error)) {
+	t.stringTransforms = append(t.stringTransforms, fn)
+}
+
+// AddObjectTransform adds a function that will be called for each object in the JSON tree
+func (t *Transformer) AddObjectTransform(fn func(path string, value map[string]any) error) {
+	t.objectTransforms = append(t.objectTransforms, fn)
+}
+
+// AddSliceTransform adds a function that will be called for each slice in the JSON tree
+func (t *Transformer) AddSliceTransform(fn func(path string, value []any) ([]any, error)) {
+	t.sliceTransforms = append(t.sliceTransforms, fn)
+}
+
+// Transform applies the transformations to the JSON tree
+func (o *Transformer) Transform(v map[string]any) error {
+	_, err := o.visitAny(v, "")
+	return err
+}
+
+// visitAny is a helper function that visits any value in the JSON tree
+func (o *Transformer) visitAny(v any, path string) (any, error) {
+	if v == nil {
+		return v, nil
+	}
+	switch v := v.(type) {
+	case map[string]any:
+		if err := o.visitMap(v, path); err != nil {
+			return nil, err
+		}
+		return v, nil
+	case []any:
+		return o.visitSlice(v, path)
+	case int64, float64, bool:
+		return o.visitPrimitive(v, path)
+	case string:
+		return o.visitString(v, path)
+	default:
+		return nil, fmt.Errorf("unhandled type at path %q: %T", path, v)
+	}
+}
+
+func (o *Transformer) visitMap(m map[string]any, path string) error {
+	for _, fn := range o.objectTransforms {
+		if err := fn(path, m); err != nil {
+			return err
+		}
+	}
+
+	for k, v := range m {
+		childPath := path + "." + k
+
+		v2, err := o.visitAny(v, childPath)
+		if err != nil {
+			return err
+		}
+		m[k] = v2
+	}
+
+	return nil
+}
+
+// visitSlice is a helper function that visits a slice in the JSON tree
+func (o *Transformer) visitSlice(s []any, path string) (any, error) {
+	for _, fn := range o.sliceTransforms {
+		var err error
+		s, err = fn(path+"[]", s)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	for i, v := range s {
+		v2, err := o.visitAny(v, path+"[]")
+		if err != nil {
+			return nil, err
+		}
+		s[i] = v2
+	}
+
+	return s, nil
+}
+
+// SortSlice sorts a slice of any values, ordered by their JSON representations.
+// This is not very efficient, but is convenient for small slice where we don't know their types.
+func SortSlice(s []any) ([]any, error) {
+	type entry struct {
+		o       any
+		sortKey string
+	}
+
+	var entries []entry
+	for i := range s {
+		j, err := json.Marshal(s[i])
+		if err != nil {
+			return nil, fmt.Errorf("error converting to json: %w", err)
+		}
+		entries = append(entries, entry{o: s[i], sortKey: string(j)})
+	}
+
+	sort.Slice(entries, func(i, j int) bool {
+		return entries[i].sortKey < entries[j].sortKey
+	})
+
+	out := make([]any, 0, len(s))
+	for i := range s {
+		out = append(out, entries[i].o)
+	}
+
+	return out, nil
+}
+
+// visitPrimitive is a helper function that visits a primitive value in the JSON tree
+func (o *Transformer) visitPrimitive(v any, _ string) (any, error) {
+	return v, nil
+}
+
+// visitString is a helper function that visits a string value in the JSON tree
+func (o *Transformer) visitString(v string, path string) (string, error) {
+	for _, fn := range o.stringTransforms {
+		var err error
+		v, err = fn(path, v)
+		if err != nil {
+			return "", err
+		}
+	}
+	return v, nil
+}

upup/pkg/fi/cloudup/awstasks/sqs.go

@@ -25,6 +25,7 @@ import (
 
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/klog/v2"
+	"k8s.io/kops/pkg/jsonutils"
 	"k8s.io/kops/upup/pkg/fi/cloudup/terraformWriter"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
@@ -115,6 +116,13 @@ func (q *SQS) Find(c *fi.CloudupContext) (*SQS, error) {
 			return nil, fmt.Errorf("error parsing actual Policy for SQS %q: %v", aws.ToString(q.Name), err)
 		}
 
+		if err := normalizePolicy(expectedJson); err != nil {
+			return nil, err
+		}
+		if err := normalizePolicy(actualJson); err != nil {
+			return nil, err
+		}
+
 		if reflect.DeepEqual(actualJson, expectedJson) {
 			klog.V(2).Infof("actual Policy was json-equal to expected; returning expected value")
 			actualPolicy = expectedPolicy
@@ -139,6 +147,47 @@ func (q *SQS) Find(c *fi.CloudupContext) (*SQS, error) {
 	return actual, nil
 }
 
+type JSONObject map[string]any
+
+func (j *JSONObject) Slice(key string) ([]any, bool, error) {
+	v, found := (*j)[key]
+	if !found {
+		return nil, false, nil
+	}
+	s, ok := v.([]any)
+	if !ok {
+		return nil, false, fmt.Errorf("expected slice at %q, got %T", key, v)
+	}
+	return s, true, nil
+}
+
+func (j *JSONObject) Object(key string) (JSONObject, bool, error) {
+	v, found := (*j)[key]
+	if !found {
+		return nil, false, nil
+	}
+	m, ok := v.(JSONObject)
+	if !ok {
+		return nil, false, fmt.Errorf("expected object at %q, got %T", key, v)
+	}
+	return m, true, nil
+}
+
+// normalizePolicy sorts the Service principals in the policy, so that we can compare policies more easily.
+func normalizePolicy(policy map[string]interface{}) error {
+	xform := jsonutils.NewTransformer()
+	xform.AddSliceTransform(func(path string, value []any) ([]any, error) {
+		if path != ".Statement[].Principal.Service[]" {
+			return value, nil
+		}
+		return jsonutils.SortSlice(value)
+	})
+	if err := xform.Transform(policy); err != nil {
+		return err
+	}
+	return nil
+}
+
 func (q *SQS) Run(c *fi.CloudupContext) error {
 	return fi.CloudupDefaultDeltaRunMethod(q, c)
 }