Time parse issue in compareTimeRanges (maintenanceWindow)

[eloo-abi]

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.

provider-aws/pkg/controller/rds/dbinstance/setup.go

Lines 590 to 616 in 4b536aa

	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, "-")
	for i := range leftSpans {
	left, err := time.Parse(format, leftSpans[i])
	if err != nil {
	return false, err
	}
	right, err := time.Parse(format, rightSpans[i])
	if err != nil {
	return false, err
	}
	if left != right {
	return true, nil
	}
	}
	return false, nil
	}

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

[github-actions]

Crossplane does not currently have enough maintainers to address every issue and pull request. This issue has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

[eloo-abi]

/fresh