crossplane-contrib / provider-gitlab

Crossplane Gitlab provider
Apache License 2.0
55 stars 30 forks source link

API Deprecations should be handled (right now group -> emails_disabled changed to emails_enabled) #141

Closed markussiebert closed 1 month ago

markussiebert commented 5 months ago

What happened?

We saw issues concerning the group api, after updating our gitlab instance. In our case, the group api deprecated the emails_disabled flag and inverted it as emails_enabled. This leads to failing group api requests of the gitlab provider.

For now we used a feature flag to disable the new behaviour. But this won't be the solution for long - I think June / the next major version might be the point were the flag could be removed.

Updating the go gitlab client from 0.86.0 to 0.103.0 would fix the gitlab api side changes, but how do we handle this customer facing? This would lead to breaking changes in the managed resource...

How can we reproduce it?

Just use the provider with an up to date gitlab instance

What environment did it happen in?

Crossplane version: not relevant Crossplane Provider GitLab version: 0.7.0

markussiebert commented 4 months ago

Any thoughts on this?

markussiebert commented 2 months ago

My thoughts on this:

I would suggest not hiding Api changes from the GitLab side and introduce a breaking change for our users. Otherwise, the maintenance of this project would increase over time. We should handle this via semver in the corresponding releases of this provider.

@MisterMX @janwillies

MisterMX commented 2 months ago

I would suggest to add a new field emails_enabled to the spec and deprecate emails_disabled but keep it around to avoid breaking changes.

Inside the controller we can set the request value to the invert of emails_disabled (if it is set), like so:

if cr.Spec.ForProvider.EmailsDisabled != nil {
    req.EmailsEnabled = !(*cr.Spec.ForProvider.EmailsDisabled)
}
if cr.Spec.ForProvider.EmailsEnabled != nil {
    req.EmailsEnabled = *cr.Spec.ForProvider.EmailsEnabled
}