Fetch package pull secrets from Crossplane SA and propagate to Provider Deployments #3332

hasheddan · 2022-09-20T02:08:32Z

Description of your changes

This changeset updates to consume package pull secrets from the Crossplane ServiceAccount, rather than the previous behavior of falling back to the default ServiceAccount. It also propagates secrets from the ServiceAccount to the ServiceAccount of any Provider Deployment. This enables installing chains of private dependencies by attaching all required package pull secrets to the Crossplane ServiceAccount.

Lastly, any package pull secrets on a ProviderRevision are propagated to their Deployment as well, but they are not propagated to dependencies. We may opt to do this in the future, but there is some subtleties around shared dependents that make it more complex to do so. Because propagating the package pull secrets from the ServiceAccount makes it possible to install chains of private dependencies, we defer propagating revision secrets to dependencies until a future date.

Fixes #3294

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

make build && make e2e.run skipcleanup=true

Then created pull secret:

up ctp pull-secret create package-pull-secret -n crossplane-system -f creds.json

Patched Crossplane SA:

kubectl patch serviceaccount crossplane -n crossplane-system -p '{"imagePullSecrets": [{"name": "package-pull-secret"}]}'

Installed provider without pull secrets:

kxp install provider xpkg.upbound.io/upbound/provider-aws:v0.13.0

Verified installed and healthy:

🤖 (crossplane) k describe provider.pkg.crossplane.io/upbound-provider-aws
Name:         upbound-provider-aws
Namespace:    
Labels:       <none>
Annotations:  <none>
API Version:  pkg.crossplane.io/v1
Kind:         Provider
Spec:
  Ignore Crossplane Constraints:  false
  Package:                        xpkg.upbound.io/upbound/provider-aws:v0.13.0
  Package Pull Policy:            IfNotPresent
  Revision Activation Policy:     Automatic
  Revision History Limit:         0
  Skip Dependency Resolution:     false
Status:
  Conditions:
    Last Transition Time:  2022-09-21T13:53:22Z
    Reason:                HealthyPackageRevision
    Status:                True
    Type:                  Healthy
    Last Transition Time:  2022-09-21T13:52:17Z
    Reason:                ActivePackageRevision
    Status:                True
    Type:                  Installed
  Current Identifier:      xpkg.upbound.io/upbound/provider-aws:v0.13.0
  Current Revision:        upbound-provider-aws-fc98ece40acb
Events:
  Type     Reason                  Age                From                                 Message
  ----     ------                  ----               ----                                 -------
  Warning  InstallPackageRevision  48s (x3 over 89s)  packages/provider.pkg.crossplane.io  current package revision health is unknown
  Normal   InstallPackageRevision  24s (x2 over 24s)  packages/provider.pkg.crossplane.io  Successfully installed package revision

hasheddan · 2022-09-20T02:11:09Z

cluster/charts/crossplane/templates/deployment.yaml

+          - name: POD_SERVICE_ACCOUNT
+            valueFrom:
+              fieldRef:
+                fieldPath: spec.serviceAccountName


Note: propagating the Crossplane SA name to the xpkg fetcher could be viewed as a breaking change (if someone was attaching package pull secrets to the default SA then it would now break). However, given that this would have to take place outside of the Crossplane install process (we only allow passing package pull secrets to Crossplane SA), it is quite unituitive behavior, and is not documented as supported, I feel comfortable including this in the upcoming minor release.

Agreed that this was an unintended behavior anyway - using secret names from an SA that's not referenced during installation of Crossplane. Worth calling out in the release notes though.

muvaf

LGTM after addressing my single comment. Thanks @hasheddan !

muvaf · 2022-09-21T10:39:08Z

internal/xpkg/fetch.go

 // NewK8sFetcher creates a new K8sFetcher.
-func NewK8sFetcher(client kubernetes.Interface, namespace string, opts ...FetcherOpt) (*K8sFetcher, error) {
+func NewK8sFetcher(client kubernetes.Interface, opts ...FetcherOpt) (*K8sFetcher, error) {


Why do we remove the namespace from arguments? Can it function without a namespace?

We are just moving to passing Namespace via option to simplify signature here. It is a similar situation as SA and will just fall back to default if not provided. (ref: https://github.com/google/go-containerregistry/blob/9a5c14ada6ffae772793231c186a50d911668e72/pkg/authn/kubernetes/keychain.go#L37)

Updates the core Crossplane chart to pass its ServiceAccount name as an environment variable via the downward API. Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

Adds a flag and uses it to set the ServiceAccount name in the pkg controllers options. Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

Updates the fetcher to store the ServiceAccount used for package pull secrets and moves to passing Namespace and ServiceAccount via options. Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

Updates all package reconciler fetchters to take a namespace and service account. Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

Passes all revision and service account pull secrets to Provider deployments. This means that every Provider deployment gets the core Crossplane secrets and the secrets of its parent revision. Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

nullable-eth

Looks good, thanks!

hasheddan requested review from a team as code owners September 20, 2022 02:08

hasheddan requested review from bobh66, nlinx and turkenh September 20, 2022 02:08

hasheddan commented Sep 20, 2022

View reviewed changes

hasheddan force-pushed the use-the-sa branch from c85a6c4 to 0d5b6ac Compare September 20, 2022 02:14

muvaf approved these changes Sep 21, 2022

View reviewed changes

hasheddan force-pushed the use-the-sa branch 2 times, most recently from 559072a to da4c2ae Compare September 21, 2022 14:12

hasheddan added 5 commits September 21, 2022 10:14

Pass Crossplane ServiceAccount name to env var via downward API

ed0f58a

Updates the core Crossplane chart to pass its ServiceAccount name as an environment variable via the downward API. Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

Add flag and pkg controller options support for ServiceAccount

c2a6242

Adds a flag and uses it to set the ServiceAccount name in the pkg controllers options. Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

Update fetcher to store ServiceAccount and use options

3552aaa

Updates the fetcher to store the ServiceAccount used for package pull secrets and moves to passing Namespace and ServiceAccount via options. Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

Pass namespace and service account to all fetchers

b700c72

Updates all package reconciler fetchters to take a namespace and service account. Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

hasheddan force-pushed the use-the-sa branch from da4c2ae to ab7ebe9 Compare September 21, 2022 14:15

nullable-eth approved these changes Sep 21, 2022

View reviewed changes

hasheddan merged commit ffa5f55 into crossplane:master Sep 21, 2022

hasheddan mentioned this pull request Jan 15, 2023

Document attaching package pull secrets to Crossplane ServiceAccount crossplane/docs#276

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fetch package pull secrets from Crossplane SA and propagate to Provider Deployments #3332

Fetch package pull secrets from Crossplane SA and propagate to Provider Deployments #3332

hasheddan commented Sep 20, 2022 •

edited

hasheddan Sep 20, 2022

muvaf Sep 21, 2022

muvaf left a comment

muvaf Sep 21, 2022

hasheddan Sep 21, 2022

nullable-eth left a comment

Fetch package pull secrets from Crossplane SA and propagate to Provider Deployments #3332

Fetch package pull secrets from Crossplane SA and propagate to Provider Deployments #3332

Conversation

hasheddan commented Sep 20, 2022 • edited

Description of your changes

How has this code been tested

hasheddan Sep 20, 2022

Choose a reason for hiding this comment

muvaf Sep 21, 2022

Choose a reason for hiding this comment

muvaf left a comment

Choose a reason for hiding this comment

muvaf Sep 21, 2022

Choose a reason for hiding this comment

hasheddan Sep 21, 2022

Choose a reason for hiding this comment

nullable-eth left a comment

Choose a reason for hiding this comment

hasheddan commented Sep 20, 2022 •

edited