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

Fix connection secret owner check for K8s Secret Store #380

Merged
merged 1 commit into from
Feb 20, 2023

Conversation

turkenh
Copy link
Member

@turkenh turkenh commented Feb 9, 2023

Description of your changes

This fixes an issue when the External Secret Store feature is enabled, but K8s secrets are used (as the default secret store).

With External Secret Stores, we use a special label/tag with the key secret.crossplane.io/owner-uid to track the owner of the connection secret. However, different from other Secret Store implementations, Kubernetes Store uses metadata.OwnerReferences for this purpose, and we don't want it to appear in the labels of the secret additionally.
We are adding the owner label to the internal representation of the current secret as part of converting store.WriteOption's to k8s resource.ApplyOption's, so that our generic store.WriteOptions checking secret owner could work as expected.

Fixes crossplane/crossplane#3520

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

  1. Install XP with the external secret store feature enabled.

  2. Apply the following XRD and Composition

    `XRD` yaml:
    apiVersion: apiextensions.crossplane.io/v1
    kind: CompositeResourceDefinition
    metadata:
      name: xcruiseserviceaccounts.getcruise.com
    spec:
      group: getcruise.com
      names:
        kind: XCruiseServiceAccount
        plural: xcruiseserviceaccounts
      claimNames:
        kind: CruiseServiceAccount
        plural: cruiseserviceaccounts
      versions:
        - name: v1alpha1
          served: true
          referenceable: true
          schema:
            openAPIV3Schema:
              type: object
              properties:
                spec: {}
    `Composition` yaml: ``` apiVersion: apiextensions.crossplane.io/v1 kind: Composition metadata: name: xcruiseserviceaccounts.getcruise.com labels: provider: gcp spec: writeConnectionSecretsToNamespace: crossplane-system compositeTypeRef: apiVersion: getcruise.com/v1alpha1 kind: XCruiseServiceAccount resources: - name: serviceaccount base: # https://doc.crds.dev/github.com/upbound/provider-gcp/cloudplatform.gcp.upbound.io/ServiceAccount/v1beta1 apiVersion: cloudplatform.gcp.upbound.io/v1beta1 kind: ServiceAccount spec: forProvider: {} patches: - type: CombineFromComposite toFieldPath: spec.forProvider.description policy: fromFieldPath: Required combine: variables: - fromFieldPath: spec.claimRef.namespace - fromFieldPath: spec.claimRef.name strategy: string string: fmt: "%s/%s" - type: ToCompositeFieldPath fromFieldPath: status.atProvider.email toFieldPath: status.serviceAccountEmail policy: fromFieldPath: Required - name: serviceaccountkey connectionDetails: - fromConnectionSecretKey: private_key base: # https://doc.crds.dev/github.com/upbound/provider-gcp/cloudplatform.gcp.upbound.io/ServiceAccountKey/v1beta1 apiVersion: cloudplatform.gcp.upbound.io/v1beta1 kind: ServiceAccountKey spec: forProvider: publicKeyType: TYPE_X509_PEM_FILE serviceAccountIdSelector: matchControllerRef: true writeConnectionSecretToRef: name: sa-key namespace: crossplane-system ```
  3. Apply the following claim

    apiVersion: getcruise.com/v1alpha1
    kind: CruiseServiceAccount
    metadata:
      name: test-sa
    spec: {}
    
  4. Verify that the composite becomes READY & SYNCED and connection secret created properly.

Fixes crossplane/crossplane#3520

Signed-off-by: Hasan Turken <turkenh@gmail.com>
Copy link
Member

@ezgidemirel ezgidemirel left a comment

Choose a reason for hiding this comment

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

Thanks @turkenh, LGTM

@turkenh
Copy link
Member Author

turkenh commented Feb 20, 2023

@hasheddan, I need an approving review here, please :)

@github-actions
Copy link

Successfully created backport PR #383 for release-0.19.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connection secret could not be written with existing secret is not owned by UID
3 participants