Order policy document sections alphabetically

kubernetes · Nov 4, 2020 · bafb2ce · bafb2ce
1 parent 850adb3
commit bafb2ce
Show file tree

Hide file tree

Showing 77 changed files with 782 additions and 766 deletions.
diff --git a/pkg/model/iam/iam_builder.go b/pkg/model/iam/iam_builder.go
@@ -48,8 +48,8 @@ const PolicyDefaultVersion = "2012-10-17"
 
 // Policy Struct is a collection of fields that form a valid AWS policy document
 type Policy struct {
-	Version   string
 	Statement []*Statement
+	Version   string
 }
 
 // AsJSON converts the policy document to JSON format (parsable by AWS)
@@ -146,6 +146,19 @@ func (s *Statement) MarshalJSON() ([]byte, error) {
 
 	jw := &jsonWriter{w: &b}
 	jw.StartObject()
+
+	if !s.Action.IsEmpty() {
+		jw.Field("Action")
+		jw.Marshal(s.Action)
+		jw.Comma()
+	}
+
+	if len(s.Condition) != 0 {
+		jw.Field("Condition")
+		jw.Marshal(s.Condition)
+		jw.Comma()
+	}
+
 	jw.Field("Effect")
 	jw.Marshal(s.Effect)
 
@@ -154,21 +167,13 @@ func (s *Statement) MarshalJSON() ([]byte, error) {
 		jw.Field("Principal")
 		jw.Marshal(s.Principal)
 	}
-	if !s.Action.IsEmpty() {
-		jw.Comma()
-		jw.Field("Action")
-		jw.Marshal(s.Action)
-	}
+
 	if !s.Resource.IsEmpty() {
 		jw.Comma()
 		jw.Field("Resource")
 		jw.Marshal(s.Resource)
 	}
-	if len(s.Condition) != 0 {
-		jw.Comma()
-		jw.Field("Condition")
-		jw.Marshal(s.Condition)
-	}
+
 	jw.EndObject()
 
 	return b.Bytes(), jw.Error()

diff --git a/pkg/model/iam/iam_builder_test.go b/pkg/model/iam/iam_builder_test.go
@@ -38,15 +38,15 @@ func TestRoundTrip(t *testing.T) {
 				Action:   stringorslice.Of("ec2:DescribeRegions"),
 				Resource: stringorslice.Of("*"),
 			},
-			JSON: "{\"Effect\":\"Allow\",\"Action\":\"ec2:DescribeRegions\",\"Resource\":\"*\"}",
+			JSON: "{\"Action\":\"ec2:DescribeRegions\",\"Effect\":\"Allow\",\"Resource\":\"*\"}",
 		},
 		{
 			IAM: &Statement{
 				Effect:   StatementEffectDeny,
 				Action:   stringorslice.Of("ec2:DescribeRegions", "ec2:DescribeInstances"),
 				Resource: stringorslice.Of("a", "b"),
 			},
-			JSON: "{\"Effect\":\"Deny\",\"Action\":[\"ec2:DescribeRegions\",\"ec2:DescribeInstances\"],\"Resource\":[\"a\",\"b\"]}",
+			JSON: "{\"Action\":[\"ec2:DescribeRegions\",\"ec2:DescribeInstances\"],\"Effect\":\"Deny\",\"Resource\":[\"a\",\"b\"]}",
 		},
 		{
 			IAM: &Statement{
@@ -56,7 +56,7 @@ func TestRoundTrip(t *testing.T) {
 					"foo": 1,
 				},
 			},
-			JSON: "{\"Effect\":\"Deny\",\"Principal\":{\"Federated\":\"federated\"},\"Condition\":{\"foo\":1}}",
+			JSON: "{\"Condition\":{\"foo\":1},\"Effect\":\"Deny\",\"Principal\":{\"Federated\":\"federated\"}}",
 		},
 		{
 			IAM: &Statement{
@@ -66,7 +66,7 @@ func TestRoundTrip(t *testing.T) {
 					"bar": "baz",
 				},
 			},
-			JSON: "{\"Effect\":\"Deny\",\"Principal\":{\"Service\":\"service\"},\"Condition\":{\"bar\":\"baz\"}}",
+			JSON: "{\"Condition\":{\"bar\":\"baz\"},\"Effect\":\"Deny\",\"Principal\":{\"Service\":\"service\"}}",
 		},
 	}
 	for _, g := range grid {

diff --git a/pkg/model/iam/tests/iam_builder_bastion.json b/pkg/model/iam/tests/iam_builder_bastion.json
@@ -1,14 +1,14 @@
 {
-  "Version": "2012-10-17",
   "Statement": [
     {
-      "Effect": "Allow",
       "Action": [
         "ec2:DescribeRegions"
       ],
+      "Effect": "Allow",
       "Resource": [
         "*"
       ]
     }
-  ]
+  ],
+  "Version": "2012-10-17"
 }
diff --git a/pkg/model/iam/tests/iam_builder_master_legacy.json b/pkg/model/iam/tests/iam_builder_master_legacy.json
@@ -1,17 +1,15 @@
 {
-  "Version": "2012-10-17",
   "Statement": [
     {
-      "Effect": "Allow",
       "Action": [
         "ec2:*"
       ],
+      "Effect": "Allow",
       "Resource": [
         "*"
       ]
     },
     {
-      "Effect": "Allow",
       "Action": [
         "autoscaling:DescribeAutoScalingGroups",
         "autoscaling:DescribeAutoScalingInstances",
@@ -22,62 +20,62 @@
         "autoscaling:UpdateAutoScalingGroup",
         "ec2:DescribeLaunchTemplateVersions"
       ],
+      "Effect": "Allow",
       "Resource": [
         "*"
       ]
     },
     {
-      "Effect": "Allow",
       "Action": [
         "elasticloadbalancing:*"
       ],
+      "Effect": "Allow",
       "Resource": [
         "*"
       ]
     },
     {
-      "Effect": "Allow",
       "Action": [
         "iam:ListServerCertificates",
         "iam:GetServerCertificate"
       ],
+      "Effect": "Allow",
       "Resource": [
         "*"
       ]
     },
     {
-      "Effect": "Allow",
       "Action": [
         "s3:*"
       ],
+      "Effect": "Allow",
       "Resource": "arn:aws:s3:::kops-tests/iam-builder-test.k8s.local/*"
     },
     {
-      "Effect": "Allow",
       "Action": [
         "s3:GetBucketLocation",
         "s3:GetEncryptionConfiguration",
         "s3:ListBucket",
         "s3:ListBucketVersions"
       ],
+      "Effect": "Allow",
       "Resource": [
         "arn:aws:s3:::kops-tests"
       ]
     },
     {
-      "Effect": "Allow",
       "Action": [
         "kms:ListGrants",
         "kms:RevokeGrant"
       ],
+      "Effect": "Allow",
       "Resource": [
         "key-id-1",
         "key-id-2",
         "key-id-3"
       ]
     },
     {
-      "Effect": "Allow",
       "Action": [
         "kms:CreateGrant",
         "kms:Decrypt",
@@ -86,23 +84,23 @@
         "kms:GenerateDataKey*",
         "kms:ReEncrypt*"
       ],
+      "Effect": "Allow",
       "Resource": [
         "key-id-1",
         "key-id-2",
         "key-id-3"
       ]
     },
     {
-      "Effect": "Allow",
       "Action": [
         "route53:ListHostedZones"
       ],
+      "Effect": "Allow",
       "Resource": [
         "*"
       ]
     },
     {
-      "Effect": "Allow",
       "Action": [
         "ecr:GetAuthorizationToken",
         "ecr:BatchCheckLayerAvailability",
@@ -112,9 +110,11 @@
         "ecr:ListImages",
         "ecr:BatchGetImage"
       ],
+      "Effect": "Allow",
       "Resource": [
         "*"
       ]
     }
-  ]
+  ],
+  "Version": "2012-10-17"
 }