Repository: crossplane/provider-aws (go)

Issue:
S3 replication metrics requires eventThreshold
### What happened?
When trying to configure an S3 bucket with the following replication spec, we encounter an error from the AWS API.

```yaml
replicationConfiguration:
  role: arn:aws:iam::<redacted>:role/<redacted>
  rules:
  - status: Enabled
     destination:
       bucket: arn:aws:s3:::<redacted>
       account: "<redacted>"
       replicationTime:
         status: Disabled
         time:
           minutes: 15
       accessControlTranslation:
         ownerOverride: Destination
       metrics:
         status: Enabled
         eventThreshold:
           minutes: 15
```

As far as I understand it, enabling `metrics`, with `replicationTime` disabled is valid configuration, the above configuration gives us the following error:

`cannot create or update: api error InvalidArgument: Metrics cannot contain an event threshold when ReplicationTime is not specified or Disabled`

However, `metrics.eventThreshold` cannot be omitted since this is a field required by the provider. 

### What environment did it happen in?
Crossplane version: `v1.6.1`
Provider version: `v0.23.0`


Pull request:
Make eventThreshold an optional field for s3 bucket replication
Signed-off-by: Alex Last <alexrichardlast@gmail.com>

### Description of your changes

Fixes #1117 

I have:

- [x] Read and followed Crossplane's [contribution process].
- [x] Run `make reviewable test` to ensure this PR is ready for review.

### How has this code been tested

- Passes all unit tests
- Deployed and tested a custom provider build on cluster and confirmed it fixes the issue


Patch:
diff --git a/apis/s3/v1beta1/replicationConfiguration_types.go b/apis/s3/v1beta1/replicationConfiguration_types.go
index c2129ad23..2bd9d8cf1 100644
--- a/apis/s3/v1beta1/replicationConfiguration_types.go
+++ b/apis/s3/v1beta1/replicationConfiguration_types.go
@@ -227,8 +227,8 @@ type EncryptionConfiguration struct {
 type Metrics struct {
 	// A container specifying the time threshold for emitting the s3:Replication:OperationMissedThreshold
 	// event.
-	// EventThreshold is a required field
-	EventThreshold ReplicationTimeValue `json:"eventThreshold"`
+	// +optional
+	EventThreshold *ReplicationTimeValue `json:"eventThreshold,omitempty"`
 
 	// Specifies whether the replication metrics are enabled.
 	//
diff --git a/apis/s3/v1beta1/zz_generated.deepcopy.go b/apis/s3/v1beta1/zz_generated.deepcopy.go
index dc02a6827..744e1550b 100644
--- a/apis/s3/v1beta1/zz_generated.deepcopy.go
+++ b/apis/s3/v1beta1/zz_generated.deepcopy.go
@@ -444,7 +444,7 @@ func (in *Destination) DeepCopyInto(out *Destination) {
 	if in.Metrics != nil {
 		in, out := &in.Metrics, &out.Metrics
 		*out = new(Metrics)
-		**out = **in
+		(*in).DeepCopyInto(*out)
 	}
 	if in.ReplicationTime != nil {
 		in, out := &in.ReplicationTime, &out.ReplicationTime
@@ -759,7 +759,11 @@ func (in *LoggingConfiguration) DeepCopy() *LoggingConfiguration {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *Metrics) DeepCopyInto(out *Metrics) {
 	*out = *in
-	out.EventThreshold = in.EventThreshold
+	if in.EventThreshold != nil {
+		in, out := &in.EventThreshold, &out.EventThreshold
+		*out = new(ReplicationTimeValue)
+		**out = **in
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Metrics.
diff --git a/package/crds/s3.aws.crossplane.io_buckets.yaml b/package/crds/s3.aws.crossplane.io_buckets.yaml
index daecd158b..f8669fd25 100644
--- a/package/crds/s3.aws.crossplane.io_buckets.yaml
+++ b/package/crds/s3.aws.crossplane.io_buckets.yaml
@@ -1074,7 +1074,7 @@ spec:
                                     eventThreshold:
                                       description: A container specifying the time
                                         threshold for emitting the s3:Replication:OperationMissedThreshold
-                                        event. EventThreshold is a required field
+                                        event.
                                       properties:
                                         minutes:
                                           description: "Contains an integer specifying
@@ -1093,7 +1093,6 @@ spec:
                                       - Disabled
                                       type: string
                                   required:
-                                  - eventThreshold
                                   - status
                                   type: object
                                 replicationTime:
diff --git a/pkg/controller/s3/bucket/replicationConfig.go b/pkg/controller/s3/bucket/replicationConfig.go
index 9a4f1fd37..6d9b535dc 100644
--- a/pkg/controller/s3/bucket/replicationConfig.go
+++ b/pkg/controller/s3/bucket/replicationConfig.go
@@ -192,6 +192,7 @@ func createReplicationRulesFromExternal(external *types.ReplicationConfiguration
 			if rule.Destination.Metrics != nil {
 				config.Rules[i].Destination.Metrics = &v1beta1.Metrics{}
 				if rule.Destination.Metrics.EventThreshold != nil {
+					config.Rules[i].Destination.Metrics.EventThreshold = &v1beta1.ReplicationTimeValue{}
 					config.Rules[i].Destination.Metrics.EventThreshold.Minutes = rule.Destination.Metrics.EventThreshold.Minutes
 				}
 				config.Rules[i].Destination.Metrics.Status = string(rule.Destination.Metrics.Status)
@@ -256,8 +257,10 @@ func copyDestination(input *v1beta1.ReplicationRule, newRule *types.ReplicationR
 	}
 	if input.Destination.Metrics != nil {
 		newRule.Destination.Metrics = &types.Metrics{
-			EventThreshold: &types.ReplicationTimeValue{Minutes: input.Destination.Metrics.EventThreshold.Minutes},
-			Status:         types.MetricsStatus(input.Destination.Metrics.Status),
+			Status: types.MetricsStatus(input.Destination.Metrics.Status),
+		}
+		if input.Destination.Metrics.EventThreshold != nil {
+			newRule.Destination.Metrics.EventThreshold = &types.ReplicationTimeValue{Minutes: input.Destination.Metrics.EventThreshold.Minutes}
 		}
 	}
 	if input.Destination.ReplicationTime != nil {


Test patch:
diff --git a/pkg/controller/s3/bucket/replicationConfig_test.go b/pkg/controller/s3/bucket/replicationConfig_test.go
index 601b1e42d..44a15619c 100644
--- a/pkg/controller/s3/bucket/replicationConfig_test.go
+++ b/pkg/controller/s3/bucket/replicationConfig_test.go
@@ -54,7 +54,7 @@ func generateReplicationConfig() *v1beta1.ReplicationConfiguration {
 				Bucket:                   &bucketName,
 				EncryptionConfiguration:  &v1beta1.EncryptionConfiguration{ReplicaKmsKeyID: &kmsID},
 				Metrics: &v1beta1.Metrics{
-					EventThreshold: v1beta1.ReplicationTimeValue{Minutes: int32(replicationTime)},
+					EventThreshold: &v1beta1.ReplicationTimeValue{Minutes: int32(replicationTime)},
 					Status:         enabled,
 				},
 				ReplicationTime: &v1beta1.ReplicationTime{
@@ -486,7 +486,7 @@ func TestIsUpToDate(t *testing.T) {
 							Bucket:                   &bucketName,
 							EncryptionConfiguration:  &v1beta1.EncryptionConfiguration{ReplicaKmsKeyID: &kmsID},
 							Metrics: &v1beta1.Metrics{
-								EventThreshold: v1beta1.ReplicationTimeValue{Minutes: int32(replicationTime)},
+								EventThreshold: &v1beta1.ReplicationTimeValue{Minutes: int32(replicationTime)},
 								Status:         enabled,
 							},
 							ReplicationTime: &v1beta1.ReplicationTime{
@@ -557,7 +557,7 @@ func TestIsUpToDate(t *testing.T) {
 							Bucket:                   awsclient.String("bucket-1"),
 							EncryptionConfiguration:  &v1beta1.EncryptionConfiguration{ReplicaKmsKeyID: &kmsID},
 							Metrics: &v1beta1.Metrics{
-								EventThreshold: v1beta1.ReplicationTimeValue{Minutes: int32(replicationTime)},
+								EventThreshold: &v1beta1.ReplicationTimeValue{Minutes: int32(replicationTime)},
 								Status:         enabled,
 							},
 							ReplicationTime: &v1beta1.ReplicationTime{
@@ -586,7 +586,7 @@ func TestIsUpToDate(t *testing.T) {
 								Bucket:                   awsclient.String("bucket-2"),
 								EncryptionConfiguration:  &v1beta1.EncryptionConfiguration{ReplicaKmsKeyID: &kmsID},
 								Metrics: &v1beta1.Metrics{
-									EventThreshold: v1beta1.ReplicationTimeValue{Minutes: int32(replicationTime)},
+									EventThreshold: &v1beta1.ReplicationTimeValue{Minutes: int32(replicationTime)},
 									Status:         enabled,
 								},
 								ReplicationTime: &v1beta1.ReplicationTime{
@@ -686,7 +686,7 @@ func TestIsUpToDate(t *testing.T) {
 							Bucket:                   &bucketName,
 							EncryptionConfiguration:  &v1beta1.EncryptionConfiguration{ReplicaKmsKeyID: &kmsID},
 							Metrics: &v1beta1.Metrics{
-								EventThreshold: v1beta1.ReplicationTimeValue{Minutes: int32(replicationTime)},
+								EventThreshold: &v1beta1.ReplicationTimeValue{Minutes: int32(replicationTime)},
 								Status:         enabled,
 							},
 							ReplicationTime: &v1beta1.ReplicationTime{
@@ -773,7 +773,7 @@ func TestIsUpToDate(t *testing.T) {
 							Bucket:                   &bucketName,
 							EncryptionConfiguration:  &v1beta1.EncryptionConfiguration{ReplicaKmsKeyID: &kmsID},
 							Metrics: &v1beta1.Metrics{
-								EventThreshold: v1beta1.ReplicationTimeValue{Minutes: int32(replicationTime)},
+								EventThreshold: &v1beta1.ReplicationTimeValue{Minutes: int32(replicationTime)},
 								Status:         enabled,
 							},
 							ReplicationTime: &v1beta1.ReplicationTime{
@@ -836,7 +836,7 @@ func TestIsUpToDate(t *testing.T) {
 							Bucket:                   &bucketName,
 							EncryptionConfiguration:  &v1beta1.EncryptionConfiguration{ReplicaKmsKeyID: &kmsID},
 							Metrics: &v1beta1.Metrics{
-								EventThreshold: v1beta1.ReplicationTimeValue{Minutes: int32(replicationTime)},
+								EventThreshold: &v1beta1.ReplicationTimeValue{Minutes: int32(replicationTime)},
 								Status:         enabled,
 							},
 							ReplicationTime: &v1beta1.ReplicationTime{