-
Notifications
You must be signed in to change notification settings - Fork 593
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 automatic ssl mode setting support #2855
Add automatic ssl mode setting support #2855
Conversation
changelog detected ✅ |
6359a4e
to
a36975d
Compare
zone.go
Outdated
// UpdateZoneAutomaticSSLSettingMode update information about Automatic SSL mode setting to the specified zone. | ||
// | ||
// API reference: https://api.cloudflare.com/[#-add-url-slug-here] | ||
func (api *API) UpdateZoneAutomaticSSLSettingMode(ctx context.Context, zoneID, mode string) (ZoneSSLSetting, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we already have a method for fetching/updating zone settings (https://github.com/cloudflare/cloudflare-go/blob/master/zone.go#L958). is there a reason we cannot reuse that here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, however, there is a slight difference in the response we return for this specific setting. As it returns a field called next_scheduled_scan
: https://github.com/cloudflare/cloudflare-go/pull/2855/files#diff-ceb3700215079abfa8dfc51b35af780ed6292f4747db0d4710c6c72de18a0a66R178
Also I think the return type here should reference the ZoneAutomaticSSLModeSetting
struct vs ZoneSSLSetting
. Will fix that typo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could always put it in a pointer with omitempty
which will conditionally set it when it is provided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the generated version of the libraries, these endpoints are flattened into a single struct so that would make sense to reflect here too if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated to use the existing Get/UpdateZoneSettings construct and added the field as part of the ZoneSetting struct with omitempty markup
This functionality has been released in v0.104.0. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
Description
This PR adds support for reading and modifying the new automatic ssl mode setting.
Has your change been tested?
Yes, there are tests in placed to verify both reading and writing the setting value.
Screenshots (if appropriate):
Types of changes
What sort of change does your code introduce/modify?
Checklist:
and relies on stable APIs.