Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add maintenance day validation for database and kubernetes clusters #1293

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

moreinhardt
Copy link
Contributor

The docs did not really tell me how to use this field.

Type: schema.TypeString,
Optional: true,
Computed: true,
ValidateFunc: validation.IsDayOfTheWeek(true),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not validate this also works with capitalized weekdays. The provider docs list lowercase examples but in the case of database clusters I tested and both were accepted by the DO API.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ignoring case here is correct, but in addition to days of the week, DOKS maintenance policies can also set any if the user has no preference.

validation.IsDayOfTheWeek is just a thin wrapper around validation.StringInSlice. So we can use that here it support any as well.

https://github.com/hashicorp/terraform-plugin-sdk/blob/00fc746ade8f31a96a9030b867bedcff3365afa5/helper/validation/time.go#L13-L24

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, I missed that even though I saw it in the docs.

I updated it and amended my commit. Hope that's alright.

@andrewsomething andrewsomething self-assigned this Dec 20, 2024
Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for another helpful contribution!

I left a comment in line re: DOKS maintenance policies also supporting any.

Type: schema.TypeString,
Optional: true,
Computed: true,
ValidateFunc: validation.IsDayOfTheWeek(true),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ignoring case here is correct, but in addition to days of the week, DOKS maintenance policies can also set any if the user has no preference.

validation.IsDayOfTheWeek is just a thin wrapper around validation.StringInSlice. So we can use that here it support any as well.

https://github.com/hashicorp/terraform-plugin-sdk/blob/00fc746ade8f31a96a9030b867bedcff3365afa5/helper/validation/time.go#L13-L24

@moreinhardt moreinhardt force-pushed the add-maintenance-day-validation branch from c831635 to c5dd54e Compare December 21, 2024 11:19
@moreinhardt moreinhardt force-pushed the add-maintenance-day-validation branch from c5dd54e to 7fe8510 Compare December 21, 2024 11:30
Type: schema.TypeString,
Optional: true,
Computed: true,
ValidateFunc: validation.StringInSlice([]string{"any", "monday", "tuesday", "wednesday", "thursday", "friday", "saturday", "sunday"}, true),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have much experience with Go. I see that Go has no line length limit. So go fmt didn't complain or change this.

Should I wrap this to have every value on a separate line for readability?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants