[josephtate]

Fixes #1587

Summary

Adds three optional fields to the api, content and worker sections of the Pulp CR:

Field	Type	Effect
extra_volumes	[]corev1.Volume	appended to the pod's volumes
extra_volume_mounts	[]corev1.VolumeMount	appended to the main container's mounts
service_account_name	string	pods run as this existing ServiceAccount instead of the operator-managed one

Unset fields leave the generated Deployments unchanged (covered by tests), so existing installs do not roll on upgrade.

spec:
  worker:
    service_account_name: pulp-worker
    extra_volumes:
    - name: signing-scripts
      secret:
        secretName: my-signing-scripts
        defaultMode: 0755
    extra_volume_mounts:
    - name: signing-scripts
      mountPath: /opt/pulp-scripts

Use case

We run Pulp on EKS with metadata/package signing. Today the operator cannot express two things we need on the worker pods, so we scale the operator-managed worker Deployment to 0 and maintain a hand-written copy:

• mount a Secret holding signing scripts (executable, defaultMode: 0755), and secrets from a CSI secret store;
• run as a ServiceAccount bound to an IAM role that can read AWS Secrets Manager.

This PR lets us drop the hand-written Deployment. It is also the Azure Key Vault CSI case from the issue.

Syntax: typed fields instead of the issue's YAML strings

The issue proposes AWX-style strings (extra_volumes: |). This PR uses typed corev1 fields instead, like env_vars, tolerations and affinity already do in this CRD:

• the API server validates them and kubectl explain documents them, so mistakes fail at kubectl apply instead of at reconcile time;
• no parsing code and no new failure mode in the reconciler;
• every volume source (csi, secret, projected, ...) works without extra code and tracks the k8s dependency.

The cost is a different syntax from the issue's literal example. The same use cases work.

Discarded approaches

• YAML-string fields (as in the issue): see above.
• Operator-created per-component ServiceAccounts with per-component annotations: more state to reconcile (create, update, delete, RBAC bindings). The user-supplied name lets the operator stay out of it.
• Name-collision validation in the operator (duplicate volume names, mount without a volume, clash with operator-managed names): the API server already rejects those Deployments with a clear error, so this would add code and status handling for no new guarantee.
• Extra mounts on init containers: deferred, can be added later without changing the API.

Notes for reviewers

• The custom ServiceAccount is not created or modified by the operator, and the operator's Role and RoleBinding stay bound to the managed one.
• Jobs (migration, admin password reset, signing) and web are unchanged.
• Add support for mounting CA certificates from ConfigMaps on vanilla K8s #1554 (mount_trusted_ca on vanilla Kubernetes with trust-manager) could be considered redundant now: env_vars plus these mounts can also deliver a CA bundle. I think it is still warranted for the trust-manager and OpenShift integration it provides. That integration is described in docs/trust-manager-integration.md, which is not linked from the docs site nav.

Review of this patch

The first version of this patch was reviewed by an independent, read-only Claude Code session (claude-sonnet-5-5), prompted to be adversarial and to assume shortcuts had been taken. I checked each finding against the code before acting on it. The patch is a single commit that I amended, so this section describes what changed after review.

What I changed, and why

1. image_pull_secrets were dropped when a custom ServiceAccount was set.

   • The operator attaches spec.image_pull_secrets to the ServiceAccount it manages (controllers/repo_manager/rbac.go), and pods get their pull secrets through it. The first version only changed ServiceAccountName in the pod spec. A user-supplied ServiceAccount replaces the managed one instead of adding to it, so pods lost their pull secrets and a private registry would have failed with ImagePullBackOff.
   • Now, when service_account_name is set, the operator copies spec.image_pull_secrets into the pod spec. Pods using the managed ServiceAccount are unchanged. A generator test covers both cases.

2. The docs and sample used a path the operator already uses.

   • They mounted the scripts Secret at /var/lib/pulp/scripts. When signing_secret is set, the operator mounts each signing script under that path (settings.SigningScriptPath), and the path sits inside the file-storage mount at /var/lib/pulp.
   • I changed the examples to /opt/pulp-scripts.

3. The docs did not say which pods use the fields. The docs page now states that jobs and the web pods do not use them.

4. The first tests could not catch a broken implementation.

   • They called the helper functions directly and ran setVolumeMounts on an empty struct. Deleting the line in setVolumes that appends the extra volumes, or reverting the ServiceAccount line in Deploy, would not have failed them.
   • I replaced them with tests that run the real Deploy path for api, content and worker (controllers/extra_volumes_test.go). I reverted the ServiceAccount, volume, mount and pull-secret lines one at a time, and each revert fails 3 to 6 tests.
   • I added the cases the first version lacked:
     • fields unset leave the Deployment unchanged;
     • no leakage between components;
     • extra mounts stay out of init containers;
     • building the Deployment does not write into the CR's slices (tested with spare slice capacity);
     • changing the ServiceAccount, a volume or a mount changes the hash label (so the Deployment rolls), and changing another component does not.
   • I also added envtest specs (controllers/repo_manager/extra_volumes_test.go) that check CRD validation against the generated CRD. There were none before.

What I did not change, and why

Finding	Why I left it
Validate volume and mount names in the operator (duplicates, a mount with no volume, clashes with operator-managed names, mount paths)	The API server already rejects these Deployments with a clear error. A validator would add precheck code and status handling, and would have to run after the Deployment is built because telemetry and LDAP volumes are appended later. It guarantees nothing the API server does not.
Add a Pattern/MaxLength to service_account_name	It would only catch malformed names. The likely mistake is a ServiceAccount that does not exist, which a pattern cannot detect, and no other name field in this CRD has one.
sa_annotations already covers the common IRSA case, so a per-component ServiceAccount is redundant	sa_annotations annotates the single managed ServiceAccount for all components. Per-component service_account_name is for when components need different identities (for example only the worker reads AWS Secrets Manager).
Deep-copy each Volume, because the generated pod spec shares nested pointers (such as defaultMode) with the CR	Nothing mutates them. The existing env_vars, tolerations and affinity handling shares them the same way.
The CRD grows from about 728 KB to about 1.1 MB	Embedding corev1.Volume for three components causes this. Avoiding it means an untyped field such as x-kubernetes-preserve-unknown-fields, which loses the validation that is the reason for choosing typed fields.
No test combining extras with mount_trusted_ca, LDAP, telemetry or signing_secret	Extras are appended before those volumes, and there is no shared state. The signing_secret path needs a Secret in the cluster, which the unit tests do not set up. This is untested.

Tests

• Generator tests (controllers/extra_volumes_test.go) run the real Deploy path for api, content and worker.
• Envtest tests (controllers/repo_manager/extra_volumes_test.go) check CRD validation: valid input is accepted and preserved, and wrong types and missing required keys are rejected.
• KUBEBUILDER_ASSETS=... go test ./... passes; make bundle and operator-sdk bundle validate pass; .ci/scripts/validate_commit_message.py passes.

Live cluster test

Ran the operator built from this branch against a throwaway kind cluster (Kubernetes v1.33.1, quay.io/pulp/pulp-minimal:stable) with a Pulp CR that set service_account_name, extra_volumes and extra_volume_mounts on worker (a Secret with defaultMode: 0755, mounted read-only) and an emptyDir on content, plus image_pull_secrets.

• The generated Deployments matched the CR. The worker ran as the custom ServiceAccount with the pull secret and the extra volume and mount. The api and web Deployments were unchanged, and the content Deployment kept the operator-managed ServiceAccount.
• All pods started and the migration job completed. In the worker pod the script was executable and ran from the mount, and the mount was read-only. The pod's ServiceAccount token identified it as system:serviceaccount:<ns>:pulp-worker. The api pod's token identified the operator-managed ServiceAccount.
• The content mount was writable. The api and content pods did not have the worker's mount.
• Changing service_account_name rolled the worker to a new pod with the new identity. Removing all three fields rolled it back to the operator-managed ServiceAccount, with no extra volume, no mount and no pod-level pull secrets.
• A service_account_name that does not exist is not caught by the operator. The ReplicaSet reports FailedCreate: error looking up service account and the existing pods keep running.

Not covered by the live test: a CSI volume (no CSI driver in the cluster; the CSI schema is covered by the envtest specs) and a real IAM-bound ServiceAccount.

AI disclosure

Per the AI policy: the code, tests, docs and this description were written with Hermes Agent running claude-sonnet-5-5 (Anthropic). The adversarial review above ran in Claude Code (claude-sonnet-5-5). The commit carries an Assisted By: label. I reviewed the code and this description and can explain the change.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: josephtate
Once this PR has been reviewed and has the lgtm label, please assign dkliban for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci]

Hi @josephtate. Thanks for your PR.

I'm waiting for a pulp member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.