Repository navigation
Conversation
Since the move to Helm, TestOperatorUpgrade installed the released operator image with the chart from the current checkout. That only works while the current chart stays compatible with the released binary: a chart that passes a new controller flag or narrows the RBAC makes the released controller crash or fail to sync, and the test never gets to the upgrade. Install the released version from its published chart instead, the way users upgrading from it have it deployed, then upgrade to the local chart as before. The chart comes from RELEASED_OPERATOR_CHART, so downstream builds can point elsewhere, and its version is RELEASED_OPERATOR_TAG without the leading "v", matching how the release workflow publishes it. The released image is now the one the chart deploys, so RELEASED_OPERATOR_IMG is gone. On bink, push-released-operator-image reads that image from the chart and still copies it to the local registry, which keeps the test fast and avoids pulls from a remote registry timing out; the test overrides the chart image with that copy. The restore cleanup is now registered right after the uninstall: when the released operator failed to come up, the cleanup was never registered and every later test ran without an operator. Assisted-by: AI Signed-off-by: Alice Frosi <afrosi@redhat.com>
Prep for a --pull-secret-namespaces flag. The operator namespace is always included so that setting the flag only adds namespaces, and the parser rejects invalid names up front: a namespaced Secret informer for one could never sync, which would leave the controller stuck at startup with an unclear error. Assisted-by: AI Signed-off-by: Alice Frosi <afrosi@redhat.com>
Anyone who can create or edit a BootcNodePool could point pullSecretRef at a Secret in any namespace and have the operator use those credentials and ship them to managed nodes. Pools may now only reference pull secrets in the namespaces given by the new --pull-secret-namespaces flag, plus the operator namespace, which is always allowed. Other pools become Degraded with reason SecretError and a message naming the namespace and the allowed ones, and no nodes are created or updated for them. This is a behavior change for pools that use a secret outside the operator namespace: after upgrading they must move the secret or have its namespace added to the flag. Assisted-by: AI Signed-off-by: Alice Frosi <afrosi@redhat.com>
Watching Secrets made the informer cache every Secret in the cluster; secretTypePredicate only filters which events trigger a reconcile, not what is stored. Now that pools can only use secrets from the allowed namespaces, there is no reason to cache anything else. This is also what lets the controller drop its cluster-wide Secret RBAC: the informer no longer lists or watches Secrets outside the allowlist. Assisted-by: AI Signed-off-by: Alice Frosi <afrosi@redhat.com>
The controller's ClusterRole let it read every Secret in the cluster, broader than ARCHITECTURE.md describes. With the Secret cache now limited to the allowed namespaces, cluster-wide access is no longer needed. Secret read access moves from the ClusterRole to a Role in the operator namespace, bound to the controller ServiceAccount, in both the Kustomize manifests and the generated Helm chart. Extra namespaces need their own Role, which the Helm chart grows next. controller-gen now emits role.yaml as two documents, a ClusterRole and a Role. sync-helm.sh read .rules from the whole file, which would merge them and put the secrets rule back into the Helm ClusterRole, so it now selects the document by kind. It also templates the namespace of the new RoleBinding, whose source carries the Kustomize "system" placeholder. Assisted-by: AI Signed-off-by: Alice Frosi <afrosi@redhat.com>
Lets pools use pull secrets from namespaces other than the release
namespace, e.g. ones owned by other teams, set at install or upgrade
time:
helm upgrade ... --set 'pullSecretNamespaces={team-a,team-b}'
The one value drives both the controller's --pull-secret-namespaces flag
(always including the release namespace) and a Role and RoleBinding in
each listed namespace, so the allowlist and the permissions cannot drift.
The namespaces must already exist; the chart does not create them. A
values schema rejects anything that is not a list of valid namespace
names, as the controller would otherwise fail to start.
Assisted-by: AI
Signed-off-by: Alice Frosi <afrosi@redhat.com>
The chart's RBAC and controller args are generated or templated from several sources, and nothing checked the rendered result, so a mistake in sync-helm.sh or the templates would only show up on a real install. make helm-test renders the chart with and without extra pullSecretNamespaces and checks the controller flag, that each namespace gets one secrets Role bound to the controller, that the controller ClusterRole has no secrets access, and that the values schema rejects bad input. helm is installed with go install like the other tools. The unit CI job runs the target. Assisted-by: AI Signed-off-by: Alice Frosi <afrosi@redhat.com>
TestOperatorUpgrade uninstalls the operator and reinstalls it with Helm, passing back the image and controller.extraArgs but no other values. On a cluster deployed with pullSecretNamespaces, the extra namespaces were dropped, so pools using secrets there broke for the rest of the run. It now reads the allowed namespaces from the controller's --pull-secret-namespaces flag before uninstalling and sets them again whenever it moves to the current chart. The released chart does not know the value, so the install of the released version does not pass it. Assisted-by: AI Signed-off-by: Alice Frosi <afrosi@redhat.com>
deploy-bink now creates an extra namespace and adds it to the release's pullSecretNamespaces, and passes its name to the tests. Configuring it at deploy time, rather than from the test, avoids restarting the controller during the run, so the new test can run in parallel. In TestPullSecretNamespaces, a pool referencing a secret in a namespace the test creates must be Degraded with SecretError and claim no node; switched to a secret in the allowed namespace, its node must update through the authenticated registry. The test is skipped if the operator does not allow the namespace, e.g. when it was deployed some other way. Assisted-by: AI Signed-off-by: Alice Frosi <afrosi@redhat.com>
The daemon will read pull secrets from a single Secret maintained by the controller and mounted into the DaemonSet, with one key per source secret. The new internal/pullsecret package holds the Secret name, the daemon mount directory and the key function, "<namespace>_<name>", so the controller and the daemon cannot drift. A namespace and a Secret name together can exceed the 253-character Secret key limit, and one invalid key would make the API server reject the whole aggregated Secret. Key returns an error in that case, and fetchPullSecretData reports it on the pool as SecretError. Assisted-by: AI Signed-off-by: Alice Frosi <afrosi@redhat.com>
The daemon will stop reading pull secrets through the API and read them from a Secret mounted into the DaemonSet instead. The new PullSecretAggregator maintains that Secret, bootc-pull-secrets in the operator namespace, with one key per source secret referenced by a pool holding the secret's .dockerconfigjson. It is the Secret's only writer, so pool reconciles cannot conflict on it. Pool, source secret and bootc-pull-secrets events all map to one request, and each reconcile recomputes the data from scratch, so stale keys disappear and a deleted or edited Secret is repaired. References the pool reconciler reports as SecretError are skipped. The aggregator also reconciles once at startup, so the Secret exists whenever the controller runs; it is kept, with no keys, when no pool references a pull secret. The controller needs create on secrets in its namespace, and update and patch on bootc-pull-secrets only. It now requires the operator namespace to be known, since that is where the Secret lives. Assisted-by: AI Signed-off-by: Alice Frosi <afrosi@redhat.com>
The DaemonSet now mounts bootc-pull-secrets, maintained by the controller, at /var/run/bootc-pull-secrets. The volume is optional, as the daemon may start before the Secret exists, and uses no subPath, so the kubelet keeps it up to date. While an update is pending, before staging, syncPullSecret copies the entry for the node's pullSecretRef to /run/ostree/auth.json instead of fetching the Secret through the API. Without a reference or a readable entry, it now removes the auth file instead of leaving stale credentials behind, and bootc falls back to /etc or /usr/lib. A missing entry needs no special handling: the kubelet may take a minute to sync a new or rotated Secret, and a failed stage is retried with backoff and re-reads the file. The daemon tests now use temporary directories for the host root and the mount, so they never touch the real /run/ostree. Assisted-by: AI
The daemon reads pull secrets from the mounted bootc-pull-secrets Secret, so it no longer needs to get Secrets through the API. Remove the rule from its ClusterRole, which allowed every node to read any Secret in the cluster, and the now unused APIReader. The kubelet still reads the mounted Secret: the node authorizer allows a node to read Secrets used by pods scheduled on it. make helm-test now checks that no ClusterRole, the daemon's included, grants access to secrets. Assisted-by: AI Signed-off-by: Alice Frosi <afrosi@redhat.com>
The controller now creates bootc-pull-secrets in the release namespace. Helm does not know about it, and helm uninstall keeps the namespace created by --create-namespace, so the Secret is left behind. Document deleting the namespace to remove it. Assisted-by: AI Signed-off-by: Alice Frosi <afrosi@redhat.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The controller now keeps one Secret,
bootc-pull-secrets, with one key (<namespace>_<name>) per pull secret a pool references, and mounts it into the daemon.The daemon copies that mounted entry to
/run/ostree/auth.jsonbefore staging and deletes the file when there is none, instead of reading Secrets through the API.This removes the daemon's cluster-wide
secretspermission, so nodes can only read the mounted Secret.Based on #273
Partially Fixes: ##272