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

feat(xrd): Add support for version deprecation #2948

Merged
merged 1 commit into from Mar 8, 2022

Conversation

MisterMX
Copy link
Contributor

@MisterMX MisterMX commented Mar 7, 2022

Description of your changes

This adds two new fields deprecated and deprecationWarning to XRDs spec.versions[] that match their CRD counterparts.

Fixes #2945

I have:

  • Read and followed Crossplane's [contribution process].
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Manually

Copy link
Member

@hasheddan hasheddan left a comment

Choose a reason for hiding this comment

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

LGTM -- I'm a fan of this change even though in practice I believe we can still only offer one XRD version at a time. Would like to get @negz thoughts before merge.

// +optional
Deprecated *bool `json:"deprecated,omitempty"`

// DeprecationWarning specifies the message that should be shown to the user when using this version.
Copy link
Member

Choose a reason for hiding this comment

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

Are these comments wrapped at 80 characters? If not could you make sure to wrap them! 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Signed-off-by: Maximilian Blatt <maximilian.blatt-extern@deutschebahn.com>
(external expert on behalf of DB Netz AG)
@MisterMX
Copy link
Contributor Author

MisterMX commented Mar 7, 2022

@hasheddan thanks for the review! I fixed the line lengths. Let me know if something is missing.

Regarding serving multiple XRD versions: AFAIK Crossplane does support multiple versions at the same time.

However, our main use case is to let users know when they use a deprecated version. In some cases we do deprecate complete XRDs so we might need this for every version.

@negz
Copy link
Member

negz commented Mar 7, 2022

AFAIK Crossplane does support multiple versions at the same time.

Kind of. It supports it as long as all the versions are identical. :) So, that said, this feature makes sense. e.g.:

  1. You start with a v1alpha1 API.
  2. You 'promote' the API by introducing an identical v1beta1 variant.
  3. You mark the v1alpha1 API deprecated.

Eventually we'll support introducing new API versions with different schemas per #2608.

@hasheddan hasheddan merged commit 6494953 into crossplane:master Mar 8, 2022
@MisterMX MisterMX mentioned this pull request Aug 22, 2022
6 tasks
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.

XRD versions do not support deprecation
3 participants