Fix default canned ACL delta false positive (#80)

Fixes aws-controllers-k8s/community#1282

Description of changes:
The custom ACL code was giving a false positive on the delta when comparing an empty `Spec.ACL` to the list of default ACL possibilities on a fresh bucket. This was causing the controller to always call `SyncACL`, throwing an error if the spec also defined properties which denied ACL use (such as `ObjectOwnership`). This pull request checks for the case where the `Spec.ACL` is nil and where the bucket has the default grants, and will ignore the delta, preventing the false positive.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: RedbackThomson

pkg/resource/bucket/acl_custom.go

@@ -151,6 +151,25 @@ func formGrantHeader(grants []*svcsdk.Grant) string {
 	return strings.Join(headers, ",")
 }
 
+// isDefaultCannedACLPossibilities determines whether the list of joined ACL
+// possibilites is the default for a bucket.
+func isDefaultCannedACLPossibilities(joinedPossibilities string) bool {
+	return matchPossibleCannedACL(CannedACLPrivate, joinedPossibilities) != nil
+}
+
+// matchPossibleCannedACL attempts to find a canned ACL string in a joined
+// list of possibilities. If any of the possibilities matches, it will be
+// returned, otherwise nil.
+func matchPossibleCannedACL(search string, joinedPossibilities string) *string {
+	splitPossibilities := strings.Split(joinedPossibilities, CannedACLJoinDelimiter)
+	for _, possible := range splitPossibilities {
+		if search == possible {
+			return &possible
+		}
+	}
+	return nil
+}
+
 // GetHeadersFromGrants will return a list of grant headers from grants
 func GetHeadersFromGrants(
 	resp *svcsdk.GetBucketAclOutput,

pkg/resource/bucket/hook.go

@@ -519,6 +519,11 @@ func customPreCompare(
 			b.ko.Spec.ACL = matchPossibleCannedACL(*a.ko.Spec.ACL, *b.ko.Spec.ACL)
 		}
 	} else {
+		// Ignore diff if possible canned ACLs are the default
+		if b.ko.Spec.ACL != nil && isDefaultCannedACLPossibilities(*b.ko.Spec.ACL) {
+			b.ko.Spec.ACL = nil
+		}
+
 		// If we are sure the grants weren't set from the header strings
 		if a.ko.Spec.GrantFullControl == nil &&
 			a.ko.Spec.GrantRead == nil &&
@@ -677,19 +682,6 @@ func (rm *resourceManager) setResourceACL(
 	ko.Spec.ACL = &joinedACLs
 }
 
-// matchPossibleCannedACL attempts to find a canned ACL string in a joined
-// list of possibilities. If any of the possibilities matches, it will be
-// returned, otherwise nil.
-func matchPossibleCannedACL(search string, joinedPossibilities string) *string {
-	splitPossibilities := strings.Split(joinedPossibilities, CannedACLJoinDelimiter)
-	for _, possible := range splitPossibilities {
-		if search == possible {
-			return &possible
-		}
-	}
-	return nil
-}
-
 func (rm *resourceManager) newGetBucketACLPayload(
 	r *resource,
 ) *svcsdk.GetBucketAclInput {