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

Allow Object to define a set of connectionDetails #99

Merged
merged 2 commits into from
Mar 8, 2023

Conversation

st3v
Copy link
Contributor

@st3v st3v commented Feb 14, 2023

Description of your changes

This PR introduces a new property under Object.spec, named connectionDetails. This new property is optional and can be used to specify individual entries for a connection secret for the given resource. This essentially mirrors what we already have for provider-helm (i.e. Release.spec.connectionDetails). As such, the changes in this PR borrow heavily from crossplane-contrib/provider-helm#73.

The main difference between the implementation in provider-helm and this, is that here we do not require the resources referred to under connectionDetails to have a certain set of annotations or labels. While Helm makes sure that all resources belonging to a given release have a common set of annotations, we cannot assume the same here. This is because we're dealing with all sorts of external resources here managed by arbitrary controllers that do not share a common practice when it comes to putting a well-known set of annotations or labels on the child resources they control.

This PR consists of two commits. The first one adds the new property and reconciliation logic, as well as a set of corresponding test cases and additions to the composition example. The second commit updates pre-existing parts of the example to make use of the recently introduced Release.spec.connectionDetails[*].skipPartOfReleaseCheck in provider-helm. This one is really just housekeeping and somewhat orthogonal to the main purpose of this PR. I'd be happy to remove it from here and submit it in a separate PR if you think that would be better. Just let me know.

Fixes #15

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

  • added new unit tests
  • ran make test
  • built the provider and installed it on my local cluster
  • successfully verified a number of manual e2e scenarios in my local cluster

@st3v
Copy link
Contributor Author

st3v commented Feb 14, 2023

fyi @turkenh

Copy link
Collaborator

@turkenh turkenh left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution @st3v 👍

Please also consider adding an example, or updating an existing one similar to here. This would make the feature more discoverable.

apis/object/v1alpha1/types.go Outdated Show resolved Hide resolved
# namespace: argocd
# fieldPath: data.password
# toConnectionSecretKey: argocd-password
- apiVersion: v1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for taking care of this 🙌

@st3v
Copy link
Contributor Author

st3v commented Feb 20, 2023

@turkenh You mean adding another example in addition to the arguably contrived one in my original commit?

connectionDetails:
- apiVersion: v1
kind: Service
name: guestbook-ui
namespace: argocd
fieldPath: spec.clusterIP
toConnectionSecretKey: host
- apiVersion: v1
kind: Service
name: guestbook-ui
namespace: argocd
fieldPath: spec.ports[0].port
toConnectionSecretKey: port

Signed-off-by: Stev Witzel <switzel@vmware.com>
Signed-off-by: Stev Witzel <switzel@vmware.com>
@turkenh
Copy link
Collaborator

turkenh commented Feb 24, 2023

@turkenh You mean adding another example in addition to the arguably contrived one in my original commit?

Yeah, to make it more discoverable, we can create a directory under example/object named connection-details for example and put an example there.

@turkenh turkenh merged commit 725baee into crossplane-contrib:main Mar 8, 2023
@schlakob
Copy link

schlakob commented May 8, 2023

Great feature! Looking forward using it! Are there any plans when this will be released?

@turkenh
Copy link
Collaborator

turkenh commented May 12, 2023

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.

Allows provider kubernetes to respect connection secret settings
3 participants