Skip to content

Drop internal/xresource - move back to pkg/resource from crossplane-runtime#6553

Merged
negz merged 1 commit intocrossplane:mainfrom
negz:less-x-treme
Jun 25, 2025
Merged

Drop internal/xresource - move back to pkg/resource from crossplane-runtime#6553
negz merged 1 commit intocrossplane:mainfrom
negz:less-x-treme

Conversation

@negz
Copy link
Copy Markdown
Member

@negz negz commented Jun 18, 2025

Description of your changes

Fixes #6505

See crossplane/crossplane-runtime#849 for related runtime change.

I've just made duplicates of the ConnectionSecretOwner interfaces from crossplane-runtime. c/c just uses those interfaces as arguments to a couple of methods, but it doesn't need the ConnectionSecretOwner to have ESS support.

The MR reconciler in runtime does still expect ConnectionSecretOwner to have ESS support, at least until we address crossplane/crossplane-runtime#842. Once we do that we could drop the ones I duplicated here, though it might actually be better to just define these simple interfaces here close to where they're used.

Either way this allows us to address #6505 without coupling it to immediately dropping ESS support from providers.

I have:

Need help with this checklist? See the cheat sheet.

Comment on lines +103 to +104
// LocalConnectionSecretFor creates a connection secret in the namespace of the
// supplied LocalConnectionSecretOwner, assumed to be of the supplied kind.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We only use this once in the whole codebase so I just duplicated it here for now. We could go back to using the one in runtime once runtime's LocalConnectionSecretOwner and ConnectionSecretOwner doesn't require the type to have ESS support.

The ConnectionSecretFor helpers were never aware of ESS in the first place, but they expect a type that satisfies ConnectionSecretOwner as an argument, and we added the ESS getters and setters to that interface.

type LocalConnectionSecretOwner interface {
resource.Object
resource.LocalConnectionSecretWriterTo
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We could go back to using the ones defined in runtime's pkg/resource once those types drop ESS support.

That said, I'm a believer in defining the interfaces you use close to where you use them. So maybe we just leave it like this. I don't think it matters that much either way.

Comment on lines +121 to +124
// ConnectionSecretFor creates a connection for the supplied
// ConnectionSecretOwner, assumed to be of the supplied kind. The secret is
// written to 'default' namespace if the ConnectionSecretOwner does not specify
// a namespace.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See comment on LocalConnectionSecretFor. Same deal here.

Comment on lines +379 to +383
// Only legacy XRs support writing connection secrets.
lcp, ok := cp.(resource.LegacyComposite)
if !ok {
return nil
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Calling attention to this since it's not boilerplate find/replace like most of the PR.

@negz negz marked this pull request as ready for review June 20, 2025 19:01
@negz negz requested review from a team and phisco as code owners June 20, 2025 19:01
@negz negz requested a review from turkenh June 20, 2025 19:01
Go back to crossplane-runtime/pkg/resource, which now has these changes.

Signed-off-by: Nic Cope <nicc@rk0n.org>
Copy link
Copy Markdown
Member

@jbw976 jbw976 left a comment

Choose a reason for hiding this comment

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

Thanks @negz!

@negz negz merged commit 106493b into crossplane:main Jun 25, 2025
15 of 17 checks passed
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.

[v2] Propagate internal/xresource changes back to crossplane-runtime

2 participants