Clean up hook code (#33)

- Re-order code in alphabetical order of fields (Logging moved to after Encryption)
- Fix const var casing
- Ensure all requests are made with context
- Change APICall verb from `UPDATED` to `UPDATE`

Author: RedbackThomson

pkg/resource/bucket/hook.go

@@ -24,19 +24,14 @@ import (
 )
 
 var (
-	DEFAULT_ACCELERATION_CONFIGURATION_STATUS = "Suspended"
-	DEFAULT_REQUEST_PAYER                     = "BucketOwner"
+	DefaultAccelerationConfigurationStatus = "Suspended"
+	DefaultRequestPayer                    = "BucketOwner"
 )
 
 func (rm *resourceManager) createPutFields(
 	ctx context.Context,
 	r *resource,
 ) error {
-	if r.ko.Spec.Logging != nil {
-		if err := rm.syncLogging(ctx, r); err != nil {
-			return err
-		}
-	}
 	if r.ko.Spec.Accelerate != nil {
 		if err := rm.syncAccelerate(ctx, r); err != nil {
 			return err
@@ -52,6 +47,11 @@ func (rm *resourceManager) createPutFields(
 			return err
 		}
 	}
+	if r.ko.Spec.Logging != nil {
+		if err := rm.syncLogging(ctx, r); err != nil {
+			return err
+		}
+	}
 	if r.ko.Spec.OwnershipControls != nil {
 		if err := rm.syncOwnershipControls(ctx, r); err != nil {
 			return err
@@ -93,11 +93,6 @@ func (rm *resourceManager) customUpdateBucket(
 
 	rm.setStatusDefaults(ko)
 
-	if delta.DifferentAt("Spec.Logging") {
-		if err := rm.syncLogging(ctx, desired); err != nil {
-			return nil, err
-		}
-	}
 	if delta.DifferentAt("Spec.Accelerate") {
 		if err := rm.syncAccelerate(ctx, desired); err != nil {
 			return nil, err
@@ -113,6 +108,11 @@ func (rm *resourceManager) customUpdateBucket(
 			return nil, err
 		}
 	}
+	if delta.DifferentAt("Spec.Logging") {
+		if err := rm.syncLogging(ctx, desired); err != nil {
+			return nil, err
+		}
+	}
 	if delta.DifferentAt("Spec.OwnershipControls") {
 		if err := rm.syncOwnershipControls(ctx, desired); err != nil {
 			return nil, err
@@ -144,12 +144,6 @@ func (rm *resourceManager) addPutFieldsToSpec(
 	r *resource,
 	ko *svcapitypes.Bucket,
 ) (err error) {
-	getLoggingResponse, err := rm.sdkapi.GetBucketLoggingWithContext(ctx, rm.newGetBucketLoggingPayload(r))
-	if err != nil {
-		return err
-	}
-	ko.Spec.Logging = rm.setResourceLogging(r, getLoggingResponse)
-
 	getAccelerateResponse, err := rm.sdkapi.GetBucketAccelerateConfigurationWithContext(ctx, rm.newGetBucketAcceleratePayload(r))
 	if err != nil {
 		return err
@@ -178,6 +172,12 @@ func (rm *resourceManager) addPutFieldsToSpec(
 	}
 	ko.Spec.Encryption = rm.setResourceEncryption(r, getEncryptionResponse)
 
+	getLoggingResponse, err := rm.sdkapi.GetBucketLoggingWithContext(ctx, rm.newGetBucketLoggingPayload(r))
+	if err != nil {
+		return err
+	}
+	ko.Spec.Logging = rm.setResourceLogging(r, getLoggingResponse)
+
 	getOwnershipControlsResponse, err := rm.sdkapi.GetBucketOwnershipControlsWithContext(ctx, rm.newGetBucketOwnershipControlsPayload(r))
 	if err != nil {
 		if awsErr, ok := ackerr.AWSError(err); ok && awsErr.Code() == "OwnershipControlsNotFoundError" {
@@ -225,12 +225,9 @@ func customPreCompare(
 	a *resource,
 	b *resource,
 ) {
-	if a.ko.Spec.Logging == nil && b.ko.Spec.Logging != nil {
-		a.ko.Spec.Logging = &svcapitypes.BucketLoggingStatus{}
-	}
 	if a.ko.Spec.Accelerate == nil && b.ko.Spec.Accelerate != nil {
 		a.ko.Spec.Accelerate = &svcapitypes.AccelerateConfiguration{
-			Status: &DEFAULT_ACCELERATION_CONFIGURATION_STATUS,
+			Status: &DefaultAccelerationConfigurationStatus,
 		}
 	}
 	if a.ko.Spec.CORS == nil && b.ko.Spec.CORS != nil {
@@ -239,12 +236,15 @@ func customPreCompare(
 	if a.ko.Spec.Encryption == nil && b.ko.Spec.Encryption != nil {
 		a.ko.Spec.Encryption = &svcapitypes.ServerSideEncryptionConfiguration{}
 	}
+	if a.ko.Spec.Logging == nil && b.ko.Spec.Logging != nil {
+		a.ko.Spec.Logging = &svcapitypes.BucketLoggingStatus{}
+	}
 	if a.ko.Spec.OwnershipControls == nil && b.ko.Spec.OwnershipControls != nil {
 		a.ko.Spec.OwnershipControls = &svcapitypes.OwnershipControls{}
 	}
 	if a.ko.Spec.RequestPayment == nil && b.ko.Spec.RequestPayment != nil {
 		a.ko.Spec.RequestPayment = &svcapitypes.RequestPaymentConfiguration{
-			Payer: &DEFAULT_REQUEST_PAYER,
+			Payer: &DefaultRequestPayer,
 		}
 	}
 	if a.ko.Spec.Tagging == nil && b.ko.Spec.Tagging != nil {
@@ -275,7 +275,7 @@ func (rm *resourceManager) newPutBucketAcceleratePayload(
 	}
 
 	if res.AccelerateConfiguration.Status == nil {
-		res.AccelerateConfiguration.SetStatus(DEFAULT_ACCELERATION_CONFIGURATION_STATUS)
+		res.AccelerateConfiguration.SetStatus(DefaultAccelerationConfigurationStatus)
 	}
 
 	return res
@@ -290,8 +290,8 @@ func (rm *resourceManager) syncAccelerate(
 	defer exit(err)
 	input := rm.newPutBucketAcceleratePayload(r)
 
-	_, err = rm.sdkapi.PutBucketAccelerateConfiguration(input)
-	rm.metrics.RecordAPICall("UPDATED", "PutBucketAccelerate", err)
+	_, err = rm.sdkapi.PutBucketAccelerateConfigurationWithContext(ctx, input)
+	rm.metrics.RecordAPICall("UPDATE", "PutBucketAccelerate", err)
 	if err != nil {
 		return err
 	}
@@ -329,8 +329,8 @@ func (rm *resourceManager) syncCORS(
 	defer exit(err)
 	input := rm.newPutBucketCORSPayload(r)
 
-	_, err = rm.sdkapi.PutBucketCors(input)
-	rm.metrics.RecordAPICall("UPDATED", "PutBucketCors", err)
+	_, err = rm.sdkapi.PutBucketCorsWithContext(ctx, input)
+	rm.metrics.RecordAPICall("UPDATE", "PutBucketCors", err)
 	if err != nil {
 		return err
 	}
@@ -368,8 +368,47 @@ func (rm *resourceManager) syncEncryption(
 	defer exit(err)
 	input := rm.newPutBucketEncryptionPayload(r)
 
-	_, err = rm.sdkapi.PutBucketEncryption(input)
-	rm.metrics.RecordAPICall("UPDATED", "PutBucketEncryption", err)
+	_, err = rm.sdkapi.PutBucketEncryptionWithContext(ctx, input)
+	rm.metrics.RecordAPICall("UPDATE", "PutBucketEncryption", err)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (rm *resourceManager) newGetBucketLoggingPayload(
+	r *resource,
+) *svcsdk.GetBucketLoggingInput {
+	res := &svcsdk.GetBucketLoggingInput{}
+	res.SetBucket(*r.ko.Spec.Name)
+	return res
+}
+
+func (rm *resourceManager) newPutBucketLoggingPayload(
+	r *resource,
+) *svcsdk.PutBucketLoggingInput {
+	res := &svcsdk.PutBucketLoggingInput{}
+	res.SetBucket(*r.ko.Spec.Name)
+	if r.ko.Spec.Logging != nil {
+		res.SetBucketLoggingStatus(rm.newBucketLoggingStatus(r))
+	} else {
+		res.SetBucketLoggingStatus(&svcsdk.BucketLoggingStatus{})
+	}
+	return res
+}
+
+func (rm *resourceManager) syncLogging(
+	ctx context.Context,
+	r *resource,
+) (err error) {
+	rlog := ackrtlog.FromContext(ctx)
+	exit := rlog.Trace("rm.syncLogging")
+	defer exit(err)
+	input := rm.newPutBucketLoggingPayload(r)
+
+	_, err = rm.sdkapi.PutBucketLoggingWithContext(ctx, input)
+	rm.metrics.RecordAPICall("UPDATE", "PutBucketLogging", err)
 	if err != nil {
 		return err
 	}
@@ -407,8 +446,8 @@ func (rm *resourceManager) syncOwnershipControls(
 	defer exit(err)
 	input := rm.newPutBucketOwnershipControlsPayload(r)
 
-	_, err = rm.sdkapi.PutBucketOwnershipControls(input)
-	rm.metrics.RecordAPICall("UPDATED", "PutBucketOwnershipControls", err)
+	_, err = rm.sdkapi.PutBucketOwnershipControlsWithContext(ctx, input)
+	rm.metrics.RecordAPICall("UPDATE", "PutBucketOwnershipControls", err)
 	if err != nil {
 		return err
 	}
@@ -436,7 +475,7 @@ func (rm *resourceManager) newPutBucketRequestPaymentPayload(
 	}
 
 	if res.RequestPaymentConfiguration.Payer == nil {
-		res.RequestPaymentConfiguration.SetPayer(DEFAULT_REQUEST_PAYER)
+		res.RequestPaymentConfiguration.SetPayer(DefaultRequestPayer)
 	}
 
 	return res
@@ -451,8 +490,8 @@ func (rm *resourceManager) syncRequestPayment(
 	defer exit(err)
 	input := rm.newPutBucketRequestPaymentPayload(r)
 
-	_, err = rm.sdkapi.PutBucketRequestPayment(input)
-	rm.metrics.RecordAPICall("UPDATED", "PutBucketRequestPayment", err)
+	_, err = rm.sdkapi.PutBucketRequestPaymentWithContext(ctx, input)
+	rm.metrics.RecordAPICall("UPDATE", "PutBucketRequestPayment", err)
 	if err != nil {
 		return err
 	}
@@ -490,8 +529,8 @@ func (rm *resourceManager) syncTagging(
 	defer exit(err)
 	input := rm.newPutBucketTaggingPayload(r)
 
-	_, err = rm.sdkapi.PutBucketTagging(input)
-	rm.metrics.RecordAPICall("UPDATED", "PutBucketTagging", err)
+	_, err = rm.sdkapi.PutBucketTaggingWithContext(ctx, input)
+	rm.metrics.RecordAPICall("UPDATE", "PutBucketTagging", err)
 	if err != nil {
 		return err
 	}
@@ -529,47 +568,8 @@ func (rm *resourceManager) syncWebsite(
 	defer exit(err)
 	input := rm.newPutBucketWebsitePayload(r)
 
-	_, err = rm.sdkapi.PutBucketWebsite(input)
-	rm.metrics.RecordAPICall("UPDATED", "PutBucketWebsite", err)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
-func (rm *resourceManager) newGetBucketLoggingPayload(
-	r *resource,
-) *svcsdk.GetBucketLoggingInput {
-	res := &svcsdk.GetBucketLoggingInput{}
-	res.SetBucket(*r.ko.Spec.Name)
-	return res
-}
-
-func (rm *resourceManager) newPutBucketLoggingPayload(
-	r *resource,
-) *svcsdk.PutBucketLoggingInput {
-	res := &svcsdk.PutBucketLoggingInput{}
-	res.SetBucket(*r.ko.Spec.Name)
-	if r.ko.Spec.Logging != nil {
-		res.SetBucketLoggingStatus(rm.newBucketLoggingStatus(r))
-	} else {
-		res.SetBucketLoggingStatus(&svcsdk.BucketLoggingStatus{})
-	}
-	return res
-}
-
-func (rm *resourceManager) syncLogging(
-	ctx context.Context,
-	r *resource,
-) (err error) {
-	rlog := ackrtlog.FromContext(ctx)
-	exit := rlog.Trace("rm.syncLogging")
-	defer exit(err)
-	input := rm.newPutBucketLoggingPayload(r)
-
-	_, err = rm.sdkapi.PutBucketLogging(input)
-	rm.metrics.RecordAPICall("UPDATED", "PutBucketLogging", err)
+	_, err = rm.sdkapi.PutBucketWebsiteWithContext(ctx, input)
+	rm.metrics.RecordAPICall("UPDATE", "PutBucketWebsite", err)
 	if err != nil {
 		return err
 	}