Repository: crossplane-contrib/provider-aws (go)

Issue:
Time parse issue in compareTimeRanges (maintenanceWindow)
Hi,

i guess i have discovered a bug in the comparison logic of the compareTimeRanges code for checking if the maintenanceWindow needs to be updated.
https://github.com/crossplane-contrib/provider-aws/blob/4b536aa882f6e4b4914c556bbfc4854ea5461a74/pkg/controller/rds/dbinstance/setup.go#L590-L616


### What happened?

The current implementation to check the timeRanges rely on the idea that the weekday in the string ("mon") is parsed into a time object. 
But according to the docs this is not the case
https://pkg.go.dev/time#Parse
> The day of the week is checked for syntax but it is otherwise ignored.

So its only compared to the hour and minute and not weekday. 

This results in a wrong comparison result to update the maintenanceWindow and thus could cause a skipped update if only the weekday of a maintenanceWindow would be changed.


### How can we reproduce it?

The following snippet will show the issue with a simplified function of the compareTimeRanges code.

https://go.dev/play/p/n2jRovBJBj5

### What environment did it happen in?
Bug in the code


Pull request description:
fix(rds): Only ignore weekday when comparing maintenanceWindow
<!--
Thank you for helping to improve Crossplane!

Please read through https://git.io/fj2m9 if this is your first time opening a
Crossplane pull request. Find us in https://slack.crossplane.io/messages/dev if
you need any help contributing.
-->

### Description of your changes

<!--
Briefly describe what this pull request does. Be sure to direct your reviewers'
attention to anything that needs special consideration.

We love pull requests that resolve an open Crossplane issue. If yours does, you
can uncomment the below line to indicate which issue your PR fixes, for example
"Fixes #500":

-->
Fixes #2009: Time parse issue in compareTimeRanges (maintenanceWindow)

I have:

- [ ] Read and followed Crossplane's [contribution process].
- [ ] Run `make reviewable test` to ensure this PR is ready for review.

### How has this code been tested

<!--
Before reviewers can be confident in the correctness of this pull request, it
needs to tested and shown to be correct. Briefly describe the testing that has
already been done or which is planned for this change.
-->
The following snippet will show the fix with a simplified function of the compareTimeRanges code.

https://go.dev/play/p/gNMLBbTgbtE 

[contribution process]: https://git.io/fj2m9


Patch:
diff --git a/OWNERS.md b/OWNERS.md
index 93adef98e..14ea22839 100644
--- a/OWNERS.md
+++ b/OWNERS.md
@@ -10,6 +10,8 @@ guidelines and responsibilities for the steering committee and maintainers.
 
 ## Maintainers
 
+* Daniel Mangum <georgedanielmangum@gmail.com> ([hasheddan](https://github.com/hasheddan))
+* Krish Chowdhary <kchowdha@redhat.com> ([krishchow](https://github.com/krishchow))
 * Carl Henrik Lunde <chlunde@gmail.com> ([chlunde](https://github.com/chlunde))
 * Maximilian Blatt ([MisterMX](https://github.com/MisterMX))
 * Ana Garlau ([anagarlau](https://github.com/anagarlau))
diff --git a/apis/s3/v1beta1/bucket_types.go b/apis/s3/v1beta1/bucket_types.go
index 2830a6d71..655b2ba67 100644
--- a/apis/s3/v1beta1/bucket_types.go
+++ b/apis/s3/v1beta1/bucket_types.go
@@ -64,14 +64,9 @@ type BucketParameters struct {
 	GrantWriteACP *string `json:"grantWriteAcp,omitempty"`
 
 	// Specifies whether you want S3 Object Lock to be enabled for the new bucket.
-	// After you enable Object Lock for a bucket, you can't disable Object Lock or suspend Versioning for that bucket
 	// +optional
 	ObjectLockEnabledForBucket *bool `json:"objectLockEnabledForBucket,omitempty"`
 
-	// Specifies the Object Lock rule for the bucket. Works only when ObjectLockEnabledForBucket is true.
-	// +optional
-	ObjectLockRule *ObjectLockRule `json:"objectLockRule,omitempty"`
-
 	// The container element for object ownership for a bucket's ownership controls.
 	// BucketOwnerPreferred - Objects uploaded to the bucket change ownership to the
 	// bucket owner if the objects are uploaded with the bucket-owner-full-control
diff --git a/apis/s3/v1beta1/object_lock_configuration_types.go b/apis/s3/v1beta1/object_lock_configuration_types.go
deleted file mode 100644
index 992ca61c8..000000000
--- a/apis/s3/v1beta1/object_lock_configuration_types.go
+++ /dev/null
@@ -1,45 +0,0 @@
-/*
-Copyright 2025 The Crossplane 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 v1beta1
-
-// ObjectLockRule the container element for an Object Lock rule.
-type ObjectLockRule struct {
-	// The container element for optionally specifying the default Object Lock
-	// retention settings for new objects placed in the specified bucket.
-	// The DefaultRetention settings require both a mode and a period.
-	// The DefaultRetention period can be either Days or Years but you must select one
-	// You cannot specify Days and Years at the same time.
-	// For more information, see Object Lock(https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-lock.html)
-	DefaultRetention *DefaultRetention `json:"defaultRetention,omitempty"`
-}
-
-// DefaultRetention the container element for the default Object Lock retention settings.
-type DefaultRetention struct {
-	// The number of days that you want to specify for the default retention period.
-	// Must be used with Mode.
-	Days *int32 `json:"days,omitempty"`
-
-	// The default Object Lock retention mode you want to apply to new objects placed
-	// in the specified bucket. Must be used with either Days or Years .
-	// Valid values are "GOVERNANCE", "COMPLIANCE"
-	// +kubebuilder:validation:Enum=GOVERNANCE;COMPLIANCE;
-	Mode string `json:"mode"`
-
-	// The number of years that you want to specify for the default retention period.
-	// Must be used with Mode .
-	Years *int32 `json:"years,omitempty"`
-}
diff --git a/apis/s3/v1beta1/zz_generated.deepcopy.go b/apis/s3/v1beta1/zz_generated.deepcopy.go
index c3bdba449..59894fa1a 100644
--- a/apis/s3/v1beta1/zz_generated.deepcopy.go
+++ b/apis/s3/v1beta1/zz_generated.deepcopy.go
@@ -205,11 +205,6 @@ func (in *BucketParameters) DeepCopyInto(out *BucketParameters) {
 		*out = new(bool)
 		**out = **in
 	}
-	if in.ObjectLockRule != nil {
-		in, out := &in.ObjectLockRule, &out.ObjectLockRule
-		*out = new(ObjectLockRule)
-		(*in).DeepCopyInto(*out)
-	}
 	if in.ObjectOwnership != nil {
 		in, out := &in.ObjectOwnership, &out.ObjectOwnership
 		*out = new(string)
@@ -428,31 +423,6 @@ func (in *Condition) DeepCopy() *Condition {
 	return out
 }
 
-// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
-func (in *DefaultRetention) DeepCopyInto(out *DefaultRetention) {
-	*out = *in
-	if in.Days != nil {
-		in, out := &in.Days, &out.Days
-		*out = new(int32)
-		**out = **in
-	}
-	if in.Years != nil {
-		in, out := &in.Years, &out.Years
-		*out = new(int32)
-		**out = **in
-	}
-}
-
-// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DefaultRetention.
-func (in *DefaultRetention) DeepCopy() *DefaultRetention {
-	if in == nil {
-		return nil
-	}
-	out := new(DefaultRetention)
-	in.DeepCopyInto(out)
-	return out
-}
-
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *DeleteMarkerReplication) DeepCopyInto(out *DeleteMarkerReplication) {
 	*out = *in
@@ -922,26 +892,6 @@ func (in *NotificationConfigurationFilter) DeepCopy() *NotificationConfiguration
 	return out
 }
 
-// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
-func (in *ObjectLockRule) DeepCopyInto(out *ObjectLockRule) {
-	*out = *in
-	if in.DefaultRetention != nil {
-		in, out := &in.DefaultRetention, &out.DefaultRetention
-		*out = new(DefaultRetention)
-		(*in).DeepCopyInto(*out)
-	}
-}
-
-// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ObjectLockRule.
-func (in *ObjectLockRule) DeepCopy() *ObjectLockRule {
-	if in == nil {
-		return nil
-	}
-	out := new(ObjectLockRule)
-	in.DeepCopyInto(out)
-	return out
-}
-
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *PaymentConfiguration) DeepCopyInto(out *PaymentConfiguration) {
 	*out = *in
diff --git a/examples/s3/bucket-object-lock.yaml b/examples/s3/bucket-object-lock.yaml
deleted file mode 100644
index 145689d30..000000000
--- a/examples/s3/bucket-object-lock.yaml
+++ /dev/null
@@ -1,15 +0,0 @@
----
-apiVersion: s3.aws.crossplane.io/v1beta1
-kind: Bucket
-metadata:
-  name: object-lock-sample
-spec:
-  forProvider:
-    locationConstraint: eu-central-2
-    objectLockEnabledForBucket: true
-    objectLockRule:
-      defaultRetention:
-        days: 7
-        mode: COMPLIANCE
-  providerConfigRef:
-    name: provider-aws
\ No newline at end of file
diff --git a/examples/wafv2/webacl.yaml b/examples/wafv2/webacl.yaml
index 8aa2309a1..7042e0493 100644
--- a/examples/wafv2/webacl.yaml
+++ b/examples/wafv2/webacl.yaml
@@ -29,7 +29,7 @@ spec:
                         }
                       },
                       "PositionalConstraint": "CONTAINS",
-                      "SearchString": "badBot",
+                      "SearchString": "YmFkQm90",
                       "TextTransformations": [
                         {
                           "Priority": 0,
@@ -47,7 +47,7 @@ spec:
                         }
                       },
                       "PositionalConstraint": "CONTAINS",
-                      "SearchString": "badBot",
+                      "SearchString": "YmFkQm90",
                       "TextTransformations": [
                         {
                           "Priority": 1,
@@ -72,7 +72,7 @@ spec:
               singleHeader:
                  name: "User-Agent"
             positionalConstraint: "CONTAINS"
-            searchString: "badBot"
+            searchString: "YmFkQm90"
             textTransformations:
               - priority: 1
                 type_: "NONE"
diff --git a/package/crds/s3.aws.crossplane.io_buckets.yaml b/package/crds/s3.aws.crossplane.io_buckets.yaml
index eb045b672..a22f9dd83 100644
--- a/package/crds/s3.aws.crossplane.io_buckets.yaml
+++ b/package/crds/s3.aws.crossplane.io_buckets.yaml
@@ -972,48 +972,9 @@ spec:
                         type: array
                     type: object
                   objectLockEnabledForBucket:
-                    description: |-
-                      Specifies whether you want S3 Object Lock to be enabled for the new bucket.
-                      After you enable Object Lock for a bucket, you can't disable Object Lock or suspend Versioning for that bucket
+                    description: Specifies whether you want S3 Object Lock to be enabled
+                      for the new bucket.
                     type: boolean
-                  objectLockRule:
-                    description: Specifies the Object Lock rule for the bucket. Works
-                      only when ObjectLockEnabledForBucket is true.
-                    properties:
-                      defaultRetention:
-                        description: |-
-                          The container element for optionally specifying the default Object Lock
-                          retention settings for new objects placed in the specified bucket.
-                          The DefaultRetention settings require both a mode and a period.
-                          The DefaultRetention period can be either Days or Years but you must select one
-                          You cannot specify Days and Years at the same time.
-                          For more information, see Object Lock(https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-lock.html)
-                        properties:
-                          days:
-                            description: |-
-                              The number of days that you want to specify for the default retention period.
-                              Must be used with Mode.
-                            format: int32
-                            type: integer
-                          mode:
-                            description: |-
-                              The default Object Lock retention mode you want to apply to new objects placed
-                              in the specified bucket. Must be used with either Days or Years .
-                              Valid values are "GOVERNANCE", "COMPLIANCE"
-                            enum:
-                            - GOVERNANCE
-                            - COMPLIANCE
-                            type: string
-                          years:
-                            description: |-
-                              The number of years that you want to specify for the default retention period.
-                              Must be used with Mode .
-                            format: int32
-                            type: integer
-                        required:
-                        - mode
-                        type: object
-                    type: object
                   objectOwnership:
                     description: |-
                       The container element for object ownership for a bucket's ownership controls.
diff --git a/pkg/clients/s3/bucket.go b/pkg/clients/s3/bucket.go
index 5fd8707eb..f0a695572 100644
--- a/pkg/clients/s3/bucket.go
+++ b/pkg/clients/s3/bucket.go
@@ -46,8 +46,6 @@ var (
 	LifecycleNotFoundErrCode = "NoSuchLifecycleConfiguration"
 	// SSENotFoundErrCode is the error code sent by AWS when the SSE config does not exist
 	SSENotFoundErrCode = "ServerSideEncryptionConfigurationNotFoundError"
-	// ObjectLockConfigurationErrCode is the error code by AWS when Object Lock configuration does not exist for a bucket.
-	ObjectLockConfigurationErrCode = "ObjectLockConfigurationNotFoundError"
 	// TaggingNotFoundErrCode is the error code sent by AWS when the tagging does not exist
 	TaggingNotFoundErrCode = "NoSuchTagSet"
 	// WebsiteNotFoundErrCode is the error code sent by AWS when the website config does not exist
@@ -117,9 +115,6 @@ type BucketClient interface {
 	PutBucketOwnershipControls(ctx context.Context, input *s3.PutBucketOwnershipControlsInput, opts ...func(*s3.Options)) (*s3.PutBucketOwnershipControlsOutput, error)
 	DeleteBucketOwnershipControls(ctx context.Context, input *s3.DeleteBucketOwnershipControlsInput, opts ...func(*s3.Options)) (*s3.DeleteBucketOwnershipControlsOutput, error)
 
-	GetObjectLockConfiguration(ctx context.Context, input *s3.GetObjectLockConfigurationInput, opts ...func(*s3.Options)) (*s3.GetObjectLockConfigurationOutput, error)
-	PutObjectLockConfiguration(ctx context.Context, input *s3.PutObjectLockConfigurationInput, opts ...func(*s3.Options)) (*s3.PutObjectLockConfigurationOutput, error)
-
 	BucketPolicyClient
 }
 
@@ -208,11 +203,6 @@ func WebsiteConfigurationNotFound(err error) bool {
 	return errors.As(err, &awsErr) && awsErr.ErrorCode() == WebsiteNotFoundErrCode
 }
 
-func ObjectLockConfigurationNotFound(err error) bool {
-	var awsErr smithy.APIError
-	return errors.As(err, &awsErr) && awsErr.ErrorCode() == ObjectLockConfigurationErrCode
-}
-
 // MethodNotSupported is parses the aws Error and validates if the method is allowed for a request
 func MethodNotSupported(err error) bool {
 	var awsErr smithy.APIError
diff --git a/pkg/clients/s3/fake/bucket.go b/pkg/clients/s3/fake/bucket.go
index 015c0d835..373d16caa 100644
--- a/pkg/clients/s3/fake/bucket.go
+++ b/pkg/clients/s3/fake/bucket.go
@@ -86,9 +86,6 @@ type MockBucketClient struct {
 	MockPutBucketOwnershipControls    func(ctx context.Context, input *s3.PutBucketOwnershipControlsInput, opts []func(*s3.Options)) (*s3.PutBucketOwnershipControlsOutput, error)
 	MockDeleteBucketOwnershipControls func(ctx context.Context, input *s3.DeleteBucketOwnershipControlsInput, opts []func(*s3.Options)) (*s3.DeleteBucketOwnershipControlsOutput, error)
 
-	MockGetObjectLockConfiguration func(ctx context.Context, input *s3.GetObjectLockConfigurationInput, opts []func(*s3.Options)) (*s3.GetObjectLockConfigurationOutput, error)
-	MockPutObjectLockConfiguration func(ctx context.Context, input *s3.PutObjectLockConfigurationInput, opts []func(*s3.Options)) (*s3.PutObjectLockConfigurationOutput, error)
-
 	MockBucketPolicyClient
 }
 
@@ -296,13 +293,3 @@ func (m MockBucketClient) PutBucketOwnershipControls(ctx context.Context, input
 func (m MockBucketClient) DeleteBucketOwnershipControls(ctx context.Context, input *s3.DeleteBucketOwnershipControlsInput, opts ...func(*s3.Options)) (*s3.DeleteBucketOwnershipControlsOutput, error) {
 	return m.MockDeleteBucketOwnershipControls(ctx, input, opts)
 }
-
-// GetObjectLockConfiguration is the fake method call to invoke the internal mock method
-func (m MockBucketClient) GetObjectLockConfiguration(ctx context.Context, input *s3.GetObjectLockConfigurationInput, opts ...func(*s3.Options)) (*s3.GetObjectLockConfigurationOutput, error) {
-	return m.MockGetObjectLockConfiguration(ctx, input, opts)
-}
-
-// PutObjectLockConfiguration is the fake method call to invoke the internal mock method
-func (m MockBucketClient) PutObjectLockConfiguration(ctx context.Context, input *s3.PutObjectLockConfigurationInput, opts ...func(*s3.Options)) (*s3.PutObjectLockConfigurationOutput, error) {
-	return m.MockPutObjectLockConfiguration(ctx, input, opts)
-}
diff --git a/pkg/controller/ecs/service/conversions.go b/pkg/controller/ecs/service/conversions.go
index 6d2aee0c1..77061826a 100644
--- a/pkg/controller/ecs/service/conversions.go
+++ b/pkg/controller/ecs/service/conversions.go
@@ -7,7 +7,7 @@ import (
 )
 
 // GenerateServiceCustom returns the current state in the form of *svcapitypes.Service with custom prarameters set
-func GenerateServiceCustom(resp *svcsdk.DescribeServicesOutput) *svcapitypes.Service { //nolint:gocyclo
+func GenerateServiceCustom(resp *svcsdk.DescribeServicesOutput) *svcapitypes.Service {
 	if len(resp.Services) != 1 {
 		return nil
 	}
@@ -42,47 +42,5 @@ func GenerateServiceCustom(resp *svcsdk.DescribeServicesOutput) *svcapitypes.Ser
 		params.CustomServiceParameters.TaskDefinition = out.TaskDefinition
 	}
 
-	// Get ServiceConnectConfiguration from last deployment
-	if len(out.Deployments) > 0 {
-		current := out.Deployments[0]
-		if current != nil && current.ServiceConnectConfiguration != nil {
-			params.ServiceConnectConfiguration = &svcapitypes.ServiceConnectConfiguration{
-				Enabled:   current.ServiceConnectConfiguration.Enabled,
-				Namespace: current.ServiceConnectConfiguration.Namespace,
-			}
-
-			if current.ServiceConnectConfiguration.LogConfiguration != nil {
-				params.ServiceConnectConfiguration.LogConfiguration = &svcapitypes.LogConfiguration{
-					LogDriver: current.ServiceConnectConfiguration.LogConfiguration.LogDriver,
-					Options:   current.ServiceConnectConfiguration.LogConfiguration.Options,
-				}
-
-				for _, so := range params.ServiceConnectConfiguration.LogConfiguration.SecretOptions {
-					params.ServiceConnectConfiguration.LogConfiguration.SecretOptions = append(params.ServiceConnectConfiguration.LogConfiguration.SecretOptions, &svcapitypes.Secret{
-						Name:      so.Name,
-						ValueFrom: so.ValueFrom,
-					})
-				}
-			}
-
-			for _, s := range current.ServiceConnectConfiguration.Services {
-				service := &svcapitypes.ServiceConnectService{
-					DiscoveryName:       s.DiscoveryName,
-					IngressPortOverride: s.IngressPortOverride,
-					PortName:            s.PortName,
-				}
-
-				for _, ca := range s.ClientAliases {
-					service.ClientAliases = append(service.ClientAliases, &svcapitypes.ServiceConnectClientAlias{
-						DNSName: ca.DnsName,
-						Port:    ca.Port,
-					})
-				}
-
-				params.ServiceConnectConfiguration.Services = append(params.ServiceConnectConfiguration.Services, service)
-			}
-		}
-	}
-
 	return service
 }
diff --git a/pkg/controller/ecs/service/setup.go b/pkg/controller/ecs/service/setup.go
index eb7783177..c105f2f69 100644
--- a/pkg/controller/ecs/service/setup.go
+++ b/pkg/controller/ecs/service/setup.go
@@ -253,19 +253,17 @@ func preUpdate(context context.Context, cr *svcapitypes.Service, obj *svcsdk.Upd
 }
 
 func (e *custom) postUpdate(context context.Context, cr *svcapitypes.Service, obj *svcsdk.UpdateServiceOutput, upd managed.ExternalUpdate, err error) (managed.ExternalUpdate, error) {
-	if obj != nil && obj.Service != nil {
-		arn := obj.Service.ServiceArn
+	arn := obj.Service.ServiceArn
 
-		if arn != nil {
-			if len(e.cache.RemoveTags) > 0 {
-				if _, err := e.client.UntagResourceWithContext(context, &svcsdk.UntagResourceInput{ResourceArn: arn, TagKeys: e.cache.RemoveTags}); err != nil {
-					return managed.ExternalUpdate{}, errors.Wrap(err, "UntagResource failed")
-				}
+	if arn != nil {
+		if len(e.cache.RemoveTags) > 0 {
+			if _, err := e.client.UntagResourceWithContext(context, &svcsdk.UntagResourceInput{ResourceArn: arn, TagKeys: e.cache.RemoveTags}); err != nil {
+				return managed.ExternalUpdate{}, errors.Wrap(err, "UntagResource failed")
 			}
-			if len(e.cache.AddTags) > 0 {
-				if _, err := e.client.TagResourceWithContext(context, &svcsdk.TagResourceInput{ResourceArn: arn, Tags: e.cache.AddTags}); err != nil {
-					return managed.ExternalUpdate{}, errors.Wrap(err, "TagResource failed")
-				}
+		}
+		if len(e.cache.AddTags) > 0 {
+			if _, err := e.client.TagResourceWithContext(context, &svcsdk.TagResourceInput{ResourceArn: arn, Tags: e.cache.AddTags}); err != nil {
+				return managed.ExternalUpdate{}, errors.Wrap(err, "TagResource failed")
 			}
 		}
 	}
diff --git a/pkg/controller/rds/dbinstance/setup.go b/pkg/controller/rds/dbinstance/setup.go
index fa1e27e61..027ebf29a 100644
--- a/pkg/controller/rds/dbinstance/setup.go
+++ b/pkg/controller/rds/dbinstance/setup.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log"
+	"regexp"
 	"sort"
 	"strconv"
 	"strings"
@@ -644,28 +645,48 @@ func createPatch(out *svcsdk.DescribeDBInstancesOutput, target *svcapitypes.DBIn
 	return patch, nil
 }
 
+func parseTimeWindowSpan(span, format string) (string, time.Time, error) {
+	// Regex: optional day, always HH:MM
+	re := regexp.MustCompile(`^\s*([A-Za-z]+:)?(\d{2}:\d{2})\s*$`)
+	matches := re.FindStringSubmatch(span)
+	if len(matches) != 3 {
+		return "", time.Time{}, errors.New("invalid time format, expected '[day:]HH:MM'")
+	}
+	day := ""
+	if matches[1] != "" {
+		day = strings.ToLower(strings.TrimSuffix(matches[1], ":"))
+	}
+	t, err := time.Parse(format, strings.TrimSpace(span))
+	if err != nil {
+		return day, time.Time{}, err
+	}
+	return day, t, nil
+}
+
 func compareTimeRanges(format string, expectedWindow *string, actualWindow *string) (bool, error) {
 	if pointer.StringValue(expectedWindow) == "" {
-		// no window to set, don't bother
 		return false, nil
 	}
 	if pointer.StringValue(actualWindow) == "" {
-		// expected is set but actual is not, so we should set it
 		return true, nil
 	}
-	// all windows here have a "-" in between two values in the expected format, so just split
 	leftSpans := strings.Split(*expectedWindow, "-")
 	rightSpans := strings.Split(*actualWindow, "-")
+
+	if len(leftSpans) != 2 || len(rightSpans) != 2 {
+		return false, errors.New("invalid time window format")
+	}
+
 	for i := range leftSpans {
-		left, err := time.Parse(format, leftSpans[i])
+		dayA, timeA, err := parseTimeWindowSpan(leftSpans[i], format)
 		if err != nil {
 			return false, err
 		}
-		right, err := time.Parse(format, rightSpans[i])
+		dayB, timeB, err := parseTimeWindowSpan(rightSpans[i], format)
 		if err != nil {
 			return false, err
 		}
-		if left != right {
+		if dayA != dayB || !timeA.Equal(timeB) {
 			return true, nil
 		}
 	}
diff --git a/pkg/controller/s3/bucket/object_lock_config.go b/pkg/controller/s3/bucket/object_lock_config.go
deleted file mode 100644
index 57c248e3f..000000000
--- a/pkg/controller/s3/bucket/object_lock_config.go
+++ /dev/null
@@ -1,146 +0,0 @@
-/*
-Copyright 2025 The Crossplane 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 bucket
-
-import (
-	"context"
-	"errors"
-
-	"github.com/aws/aws-sdk-go-v2/aws"
-	awss3 "github.com/aws/aws-sdk-go-v2/service/s3"
-	"github.com/aws/aws-sdk-go-v2/service/s3/types"
-	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
-	"github.com/crossplane/crossplane-runtime/pkg/meta"
-	"github.com/crossplane/crossplane-runtime/pkg/resource"
-	"github.com/google/go-cmp/cmp"
-	"github.com/google/go-cmp/cmp/cmpopts"
-
-	"github.com/crossplane-contrib/provider-aws/apis/s3/v1beta1"
-	"github.com/crossplane-contrib/provider-aws/pkg/clients/s3"
-	errorutils "github.com/crossplane-contrib/provider-aws/pkg/utils/errors"
-	"github.com/crossplane-contrib/provider-aws/pkg/utils/pointer"
-)
-
-const (
-	errMsgObjectLockIsNotDisableable    = "after you enable object lock for a bucket, you can't disable object lock that bucket"
-	errObjectLockConfigurationGetFailed = "cannot get object lock configuration"
-	errObjectLockConfigurationPutFailed = "cannot put object lock configuration"
-)
-
-type objectLockConfigurationCache struct {
-	objectLockConfiguration *types.ObjectLockConfiguration
-}
-
-// ObjectLockConfigurationClient is the client for API methods and reconciling the ObjectLockConfiguration
-type ObjectLockConfigurationClient struct {
-	client s3.BucketClient
-	cache  objectLockConfigurationCache
-}
-
-// NewObjectLockConfigurationClient creates the client for Object Lock Configuration
-func NewObjectLockConfigurationClient(client s3.BucketClient) *ObjectLockConfigurationClient {
-	return &ObjectLockConfigurationClient{client: client}
-}
-
-// Observe checks if the resource exists and if it matches the local configuration
-func (in *ObjectLockConfigurationClient) Observe(ctx context.Context, cr *v1beta1.Bucket) (ResourceStatus, error) {
-	response, err := in.client.GetObjectLockConfiguration(ctx, &awss3.GetObjectLockConfigurationInput{
-		Bucket: pointer.ToOrNilIfZeroValue(meta.GetExternalName(cr)),
-	})
-	if err != nil {
-		if s3.ObjectLockConfigurationNotFound(err) && (cr.Spec.ForProvider.ObjectLockEnabledForBucket == nil || !*cr.Spec.ForProvider.ObjectLockEnabledForBucket) {
-			return Updated, nil
-		}
-		return NeedsUpdate, errorutils.Wrap(resource.Ignore(s3.ObjectLockConfigurationNotFound, err), errObjectLockConfigurationGetFailed)
-	}
-	objectLockConfiguration := &types.ObjectLockConfiguration{
-		ObjectLockEnabled: "Disabled",
-	}
-	if response != nil && response.ObjectLockConfiguration != nil {
-		objectLockConfiguration = response.ObjectLockConfiguration
-	}
-	in.cache.objectLockConfiguration = objectLockConfiguration
-	return isUpToDate(&cr.Spec.ForProvider, objectLockConfiguration)
-}
-
-// isUpToDate compares the local configuration with the remote configuration and determines if an update is needed
-func isUpToDate(cr *v1beta1.BucketParameters, resp *types.ObjectLockConfiguration) (ResourceStatus, error) {
-	cmpOpts := []cmp.Option{cmpopts.IgnoreFields(types.ObjectLockConfiguration{}, "noSmithyDocumentSerde",
-		"Rule.noSmithyDocumentSerde", "Rule.DefaultRetention.noSmithyDocumentSerde")}
-	if !cmp.Equal(*GenerateAWSObjectLockConfiguration(cr), *resp, cmpOpts...) {
-		return NeedsUpdate, nil
-	}
-	return Updated, nil
-}
-
-// CreateOrUpdate updates or creates the Object Lock configuration on AWS side
-func (in *ObjectLockConfigurationClient) CreateOrUpdate(ctx context.Context, cr *v1beta1.Bucket) error {
-	if in.cache.objectLockConfiguration != nil {
-		if in.cache.objectLockConfiguration.ObjectLockEnabled == "Enabled" && !*cr.Spec.ForProvider.ObjectLockEnabledForBucket {
-			err := errorutils.Wrap(errors.New(errMsgObjectLockIsNotDisableable), errObjectLockConfigurationPutFailed)
-			cr.SetConditions(xpv1.ReconcileError(errors.New(errMsgObjectLockIsNotDisableable)))
-			return err
-		}
-	}
-	input := &awss3.PutObjectLockConfigurationInput{
-		Bucket:                  aws.String(meta.GetExternalName(cr)),
-		ObjectLockConfiguration: GenerateAWSObjectLockConfiguration(&cr.Spec.ForProvider),
-	}
-	_, err := in.client.PutObjectLockConfiguration(ctx, input)
-	if err != nil {
-		cr.SetConditions(xpv1.ReconcileError(err))
-		return errorutils.Wrap(err, errObjectLockConfigurationPutFailed)
-	}
-	return nil
-}
-
-// Delete does nothing because after you enable Object Lock for a bucket, you can't disable it anymore
-func (in *ObjectLockConfigurationClient) Delete(_ context.Context, _ *v1beta1.Bucket) error {
-	return nil
-}
-
-// LateInitialize is not needed, because if something is not specified in the current state, it should be deleted on aws side
-func (in *ObjectLockConfigurationClient) LateInitialize(_ context.Context, _ *v1beta1.Bucket) error {
-	return nil
-}
-
-// SubresourceExists checks if the subresource exists
-func (in *ObjectLockConfigurationClient) SubresourceExists(bucket *v1beta1.Bucket) bool {
-	return bucket.Spec.ForProvider.ObjectLockEnabledForBucket != nil
-}
-
-// GenerateAWSObjectLockConfiguration generates the AWS S3 Object Lock Configuration
-func GenerateAWSObjectLockConfiguration(in *v1beta1.BucketParameters) *types.ObjectLockConfiguration {
-	objectLockConfiguration := &types.ObjectLockConfiguration{
-		ObjectLockEnabled: "Disabled",
-	}
-	if in.ObjectLockEnabledForBucket != nil {
-		if *in.ObjectLockEnabledForBucket {
-			objectLockConfiguration.ObjectLockEnabled = "Enabled"
-		}
-	}
-	if in.ObjectLockRule != nil {
-		objectLockConfiguration.Rule = &types.ObjectLockRule{
-			DefaultRetention: &types.DefaultRetention{
-				Days:  in.ObjectLockRule.DefaultRetention.Days,
-				Mode:  types.ObjectLockRetentionMode(in.ObjectLockRule.DefaultRetention.Mode),
-				Years: in.ObjectLockRule.DefaultRetention.Years,
-			},
-		}
-	}
-	return objectLockConfiguration
-}
diff --git a/pkg/controller/s3/bucket/subresources.go b/pkg/controller/s3/bucket/subresources.go
index a4294ceeb..462fd78d2 100644
--- a/pkg/controller/s3/bucket/subresources.go
+++ b/pkg/controller/s3/bucket/subresources.go
@@ -50,7 +50,6 @@ func NewSubresourceClients(client s3.BucketClient) []SubresourceClient {
 		NewWebsiteConfigurationClient(client),
 		NewPublicAccessBlockClient(client),
 		NewPolicyClient(client),
-		NewObjectLockConfigurationClient(client),
 	}
 }
 
diff --git a/pkg/controller/s3/bucket/tagging_config.go b/pkg/controller/s3/bucket/taggingConfig.go
similarity index 97%
rename from pkg/controller/s3/bucket/tagging_config.go
rename to pkg/controller/s3/bucket/taggingConfig.go
index e4d186685..ff6215660 100644
--- a/pkg/controller/s3/bucket/tagging_config.go
+++ b/pkg/controller/s3/bucket/taggingConfig.go
@@ -41,14 +41,14 @@ const (
 	taggingDeleteFailed = "cannot delete Bucket tagging set"
 )
 
-type taggingConfigurationCache struct {
+type cache struct {
 	getBucketTaggingOutput *awss3.GetBucketTaggingOutput
 }
 
 // TaggingConfigurationClient is the client for API methods and reconciling the CORSConfiguration
 type TaggingConfigurationClient struct {
 	client s3.BucketClient
-	cache  taggingConfigurationCache
+	cache  cache
 }
 
 // NewTaggingConfigurationClient creates the client for CORS Configuration
@@ -57,7 +57,7 @@ func NewTaggingConfigurationClient(client s3.BucketClient) *TaggingConfiguration
 }
 
 // CacheBucketTaggingOutput returns cached *awss3.GetBucketTaggingOutput` if it exists, otherwise adds
-// `TaggingConfigurationClient.GetBucketTagging` output to taggingConfigurationCache and then returns it
+// `TaggingConfigurationClient.GetBucketTagging` output to cache and then returns it
 func (in *TaggingConfigurationClient) CacheBucketTaggingOutput(ctx context.Context, bucketName *string) (*awss3.GetBucketTaggingOutput, error) {
 	if in.cache.getBucketTaggingOutput == nil {
 		external, err := in.client.GetBucketTagging(ctx, &awss3.GetBucketTaggingInput{Bucket: bucketName})
diff --git a/pkg/controller/secretsmanager/secret/setup.go b/pkg/controller/secretsmanager/secret/setup.go
index ad5ef86f2..74d185605 100644
--- a/pkg/controller/secretsmanager/secret/setup.go
+++ b/pkg/controller/secretsmanager/secret/setup.go
@@ -248,7 +248,7 @@ func (e *hooks) isUpToDate(ctx context.Context, cr *svcapitypes.Secret, resp *sv
 		return false, "", nil
 	}
 	add, remove := DiffTags(cr.Spec.ForProvider.Tags, resp.Tags)
-	if len(add) != 0 || len(remove) != 0 {
+	if len(add) != 0 && len(remove) != 0 {
 		return false, "", nil
 	}
 


Test patch:
diff --git a/pkg/controller/s3/bucket/bucket_test.go b/pkg/controller/s3/bucket/bucket_test.go
index f1dac7628..bc1cb28dd 100644
--- a/pkg/controller/s3/bucket/bucket_test.go
+++ b/pkg/controller/s3/bucket/bucket_test.go
@@ -291,7 +291,7 @@ func TestCreate(t *testing.T) {
 		args
 		want
 	}{
-		"ValidInput": {
+		"VaildInput": {
 			args: args{
 				kube: &test.MockClient{
 					MockUpdate: test.NewMockUpdateFn(nil),
diff --git a/pkg/controller/s3/bucket/object_lock_config_test.go b/pkg/controller/s3/bucket/object_lock_config_test.go
deleted file mode 100644
index 108b2a494..000000000
--- a/pkg/controller/s3/bucket/object_lock_config_test.go
+++ /dev/null
@@ -1,235 +0,0 @@
-/*
-Copyright 2025 The Crossplane 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 bucket
-
-import (
-	"context"
-	"testing"
-
-	awss3 "github.com/aws/aws-sdk-go-v2/service/s3"
-	"github.com/aws/aws-sdk-go-v2/service/s3/types"
-	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/smithy-go"
-	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
-	"github.com/crossplane/crossplane-runtime/pkg/resource"
-	"github.com/crossplane/crossplane-runtime/pkg/test"
-	"github.com/google/go-cmp/cmp"
-	"github.com/pkg/errors"
-
-	"github.com/crossplane-contrib/provider-aws/apis/s3/v1beta1"
-	clients3 "github.com/crossplane-contrib/provider-aws/pkg/clients/s3"
-	"github.com/crossplane-contrib/provider-aws/pkg/clients/s3/fake"
-	s3Testing "github.com/crossplane-contrib/provider-aws/pkg/controller/s3/testing"
-	errorutils "github.com/crossplane-contrib/provider-aws/pkg/utils/errors"
-)
-
-func TestObjectLockConfigObserve(t *testing.T) {
-	type args struct {
-		cl *ObjectLockConfigurationClient
-		cr *v1beta1.Bucket
-	}
-
-	type want struct {
-		status ResourceStatus
-		err    error
-	}
-
-	cases := map[string]struct {
-		args
-		want
-	}{
-		"ObjectLockIsNotSet": {
-			args: args{
-				cr: s3Testing.Bucket(),
-				cl: NewObjectLockConfigurationClient(fake.MockBucketClient{
-					MockGetObjectLockConfiguration: func(ctx context.Context, input *awss3.GetObjectLockConfigurationInput, opts []func(*awss3.Options)) (*awss3.GetObjectLockConfigurationOutput, error) {
-						return nil, &smithy.GenericAPIError{Code: clients3.ObjectLockConfigurationErrCode}
-					},
-				}),
-			},
-			want: want{
-				status: Updated,
-				err:    nil,
-			},
-		},
-		"ObjectLockExplicitlyFalse": {
-			args: args{
-				cr: s3Testing.Bucket(s3Testing.WithObjectLockEnabledForBucket(false)),
-				cl: NewObjectLockConfigurationClient(fake.MockBucketClient{
-					MockGetObjectLockConfiguration: func(ctx context.Context, input *awss3.GetObjectLockConfigurationInput, opts []func(*awss3.Options)) (*awss3.GetObjectLockConfigurationOutput, error) {
-						return nil, &smithy.GenericAPIError{Code: clients3.ObjectLockConfigurationErrCode}
-					},
-				}),
-			},
-			want: want{
-				status: Updated,
-				err:    nil,
-			},
-		},
-		"ObjectLockSetToFalseButWasEnabledBefore": {
-			args: args{
-				cr: s3Testing.Bucket(s3Testing.WithObjectLockEnabledForBucket(false)),
-				cl: NewObjectLockConfigurationClient(fake.MockBucketClient{
-					MockGetObjectLockConfiguration: func(ctx context.Context, input *awss3.GetObjectLockConfigurationInput, opts []func(*awss3.Options)) (*awss3.GetObjectLockConfigurationOutput, error) {
-						return &awss3.GetObjectLockConfigurationOutput{ObjectLockConfiguration: &types.ObjectLockConfiguration{ObjectLockEnabled: "Enabled"}}, nil
-					},
-				}),
-			},
-			want: want{
-				status: NeedsUpdate,
-				err:    nil,
-			},
-		},
-		"ObjectLockSetToTrue": {
-			args: args{
-				cr: s3Testing.Bucket(s3Testing.WithObjectLockEnabledForBucket(true)),
-				cl: NewObjectLockConfigurationClient(fake.MockBucketClient{
-					MockGetObjectLockConfiguration: func(ctx context.Context, input *awss3.GetObjectLockConfigurationInput, opts []func(*awss3.Options)) (*awss3.GetObjectLockConfigurationOutput, error) {
-						return &awss3.GetObjectLockConfigurationOutput{ObjectLockConfiguration: &types.ObjectLockConfiguration{ObjectLockEnabled: "Disabled"}}, nil
-					},
-				}),
-			},
-			want: want{
-				status: NeedsUpdate,
-				err:    nil,
-			},
-		},
-		"ObjectLockRuleIsSetButObjectLockNotEnabled": {
-			args: args{
-				cr: s3Testing.Bucket(s3Testing.WithObjectLockRule(&v1beta1.ObjectLockRule{DefaultRetention: &v1beta1.DefaultRetention{Mode: "GOVERNANCE", Days: aws.Int32(7)}})),
-				cl: NewObjectLockConfigurationClient(fake.MockBucketClient{
-					MockGetObjectLockConfiguration: func(ctx context.Context, input *awss3.GetObjectLockConfigurationInput, opts []func(*awss3.Options)) (*awss3.GetObjectLockConfigurationOutput, error) {
-						return nil, &smithy.GenericAPIError{Code: clients3.ObjectLockConfigurationErrCode}
-					},
-				}),
-			},
-			want: want{
-				status: Updated,
-				err:    nil,
-			},
-		},
-		"ObjectLockRuleIsSet": {
-			args: args{
-				cr: s3Testing.Bucket(
-					s3Testing.WithObjectLockEnabledForBucket(true),
-					s3Testing.WithObjectLockRule(&v1beta1.ObjectLockRule{DefaultRetention: &v1beta1.DefaultRetention{Mode: "GOVERNANCE", Days: aws.Int32(7)}}),
-				),
-				cl: NewObjectLockConfigurationClient(fake.MockBucketClient{
-					MockGetObjectLockConfiguration: func(ctx context.Context, input *awss3.GetObjectLockConfigurationInput, opts []func(*awss3.Options)) (*awss3.GetObjectLockConfigurationOutput, error) {
-						return nil, &smithy.GenericAPIError{Code: clients3.ObjectLockConfigurationErrCode}
-					},
-				}),
-			},
-			want: want{
-				status: NeedsUpdate,
-				err:    nil,
-			},
-		},
-	}
-
-	for name, tc := range cases {
-		t.Run(name, func(t *testing.T) {
-			got, err := tc.args.cl.Observe(context.Background(), tc.args.cr)
-			if diff := cmp.Diff(tc.want.status, got); diff != "" {
-				t.Errorf("r: -want, +got:\n%s", diff)
-			}
-			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
-				t.Errorf("r: -want, +got:\n%s", diff)
-			}
-		})
-	}
-}
-
-func TestObjectLockConfigCreateOrUpdate(t *testing.T) {
-	type args struct {
-		cl *ObjectLockConfigurationClient
-		cr *v1beta1.Bucket
-	}
-
-	type want struct {
-		cr  resource.Managed
-		err error
-	}
-
-	cases := map[string]struct {
-		args
-		want
-	}{
-		"ObjectLockSetToFalseButWasEnabledBefore": {
-			args: args{
-				cr: s3Testing.Bucket(s3Testing.WithObjectLockEnabledForBucket(false)),
-				cl: &ObjectLockConfigurationClient{
-					cache: objectLockConfigurationCache{
-						objectLockConfiguration: &types.ObjectLockConfiguration{
-							ObjectLockEnabled: "Enabled",
-						},
-					},
-				},
-			},
-			want: want{
-				err: errorutils.Wrap(errors.New(errMsgObjectLockIsNotDisableable), errObjectLockConfigurationPutFailed),
-				cr: s3Testing.Bucket(
-					s3Testing.WithObjectLockEnabledForBucket(false),
-					s3Testing.WithConditions(xpv1.ReconcileError(errors.New(errMsgObjectLockIsNotDisableable))),
-				),
-			},
-		},
-		"ObjectLockAPIErrorResponse": {
-			args: args{
-				cr: s3Testing.Bucket(s3Testing.WithObjectLockEnabledForBucket(true)),
-				cl: NewObjectLockConfigurationClient(fake.MockBucketClient{
-					MockPutObjectLockConfiguration: func(ctx context.Context, input *awss3.PutObjectLockConfigurationInput, opts []func(*awss3.Options)) (*awss3.PutObjectLockConfigurationOutput, error) {
-						return &awss3.PutObjectLockConfigurationOutput{}, errBoom
-					},
-				}),
-			},
-			want: want{
-				err: errorutils.Wrap(errBoom, errObjectLockConfigurationPutFailed),
-				cr: s3Testing.Bucket(
-					s3Testing.WithObjectLockEnabledForBucket(true),
-					s3Testing.WithConditions(xpv1.ReconcileError(errBoom)),
-				),
-			},
-		},
-		"ObjectLockSetToTrue": {
-			args: args{
-				cr: s3Testing.Bucket(s3Testing.WithObjectLockEnabledForBucket(true)),
-				cl: NewObjectLockConfigurationClient(fake.MockBucketClient{
-					MockPutObjectLockConfiguration: func(ctx context.Context, input *awss3.PutObjectLockConfigurationInput, opts []func(*awss3.Options)) (*awss3.PutObjectLockConfigurationOutput, error) {
-						return &awss3.PutObjectLockConfigurationOutput{}, nil
-					},
-				}),
-			},
-			want: want{
-				err: nil,
-				cr:  s3Testing.Bucket(s3Testing.WithObjectLockEnabledForBucket(true)),
-			},
-		},
-	}
-
-	for name, tc := range cases {
-		t.Run(name, func(t *testing.T) {
-			err := tc.args.cl.CreateOrUpdate(context.Background(), tc.args.cr)
-			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
-				t.Errorf("r: -want, +got:\n%s", diff)
-			}
-			if diff := cmp.Diff(tc.want.cr, tc.args.cr, test.EquateConditions()); diff != "" {
-				t.Errorf("r: -want, +got:\n%s", diff)
-			}
-		})
-	}
-}
diff --git a/pkg/controller/s3/bucket/tagging_config_test.go b/pkg/controller/s3/bucket/taggingConfig_test.go
similarity index 100%
rename from pkg/controller/s3/bucket/tagging_config_test.go
rename to pkg/controller/s3/bucket/taggingConfig_test.go
diff --git a/pkg/controller/s3/testing/client_helper.go b/pkg/controller/s3/testing/clientHelper.go
similarity index 94%
rename from pkg/controller/s3/testing/client_helper.go
rename to pkg/controller/s3/testing/clientHelper.go
index 4985d5316..fea37c7bd 100644
--- a/pkg/controller/s3/testing/client_helper.go
+++ b/pkg/controller/s3/testing/clientHelper.go
@@ -83,12 +83,6 @@ func Client(m ...ClientModifier) *fake.MockBucketClient {
 				return &awss3.DeleteBucketPolicyOutput{}, nil
 			},
 		},
-		MockGetObjectLockConfiguration: func(ctx context.Context, input *awss3.GetObjectLockConfigurationInput, opts []func(*awss3.Options)) (*awss3.GetObjectLockConfigurationOutput, error) {
-			return &awss3.GetObjectLockConfigurationOutput{}, nil
-		},
-		MockPutObjectLockConfiguration: func(ctx context.Context, input *awss3.PutObjectLockConfigurationInput, opts []func(*awss3.Options)) (*awss3.PutObjectLockConfigurationOutput, error) {
-			return &awss3.PutObjectLockConfigurationOutput{}, nil
-		},
 	}
 	for _, v := range m {
 		v(client)
diff --git a/pkg/controller/s3/testing/test_helper.go b/pkg/controller/s3/testing/testHelper.go
similarity index 87%
rename from pkg/controller/s3/testing/test_helper.go
rename to pkg/controller/s3/testing/testHelper.go
index 1aba5f2a7..dd2a8255b 100644
--- a/pkg/controller/s3/testing/test_helper.go
+++ b/pkg/controller/s3/testing/testHelper.go
@@ -34,6 +34,7 @@ var (
 	grantReadACP     = "readACPGrant"
 	grantWrite       = "writeGrant"
 	grantWriteACP    = "writeACPGrant"
+	objectLock       = true
 	// BucketName is the name of the s3 bucket in testing
 	BucketName = "test.bucket.name"
 )
@@ -122,28 +123,19 @@ func WithPolicyUpdatePolicy(s *v1beta1.BucketPolicyUpdatePolicy) BucketModifier
 	return func(r *v1beta1.Bucket) { r.Spec.ForProvider.PolicyUpdatePolicy = s }
 }
 
-// WithObjectLockEnabledForBucket sets ObjectLockEnabledForBucket for an S3 Bucket
-func WithObjectLockEnabledForBucket(s bool) BucketModifier {
-	return func(r *v1beta1.Bucket) { r.Spec.ForProvider.ObjectLockEnabledForBucket = &s }
-}
-
-// WithObjectLockRule sets ObjectLockRule for an S3 Bucket
-func WithObjectLockRule(s *v1beta1.ObjectLockRule) BucketModifier {
-	return func(r *v1beta1.Bucket) { r.Spec.ForProvider.ObjectLockRule = s }
-}
-
 // Bucket creates a v1beta1 Bucket for use in testing
 func Bucket(m ...BucketModifier) *v1beta1.Bucket {
 	cr := &v1beta1.Bucket{
 		Spec: v1beta1.BucketSpec{
 			ForProvider: v1beta1.BucketParameters{
-				ACL:                &acl,
-				LocationConstraint: Region,
-				GrantFullControl:   &grantFullControl,
-				GrantRead:          &grantRead,
-				GrantReadACP:       &grantReadACP,
-				GrantWrite:         &grantWrite,
-				GrantWriteACP:      &grantWriteACP,
+				ACL:                        &acl,
+				LocationConstraint:         Region,
+				GrantFullControl:           &grantFullControl,
+				GrantRead:                  &grantRead,
+				GrantReadACP:               &grantReadACP,
+				GrantWrite:                 &grantWrite,
+				GrantWriteACP:              &grantWriteACP,
+				ObjectLockEnabledForBucket: &objectLock,
 			},
 		},
 	}