[FSI] Image registry policy — clean baseline (no registry exceptions) - #5126
Closed
Anthony Vollmer (avollmer-redhat) wants to merge 14 commits into
Closed
[FSI] Image registry policy — clean baseline (no registry exceptions)#5126Anthony Vollmer (avollmer-redhat) wants to merge 14 commits into
Anthony Vollmer (avollmer-redhat) wants to merge 14 commits into
Conversation
openshift-ci
Bot
requested review from
Gerd Oberlechner (geoberle) and
Miquel Simon (miquelsi)
May 5, 2026 13:19
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: avollmer-redhat The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Copilot started reviewing on behalf of
Anthony Vollmer (avollmer-redhat)
May 5, 2026 13:20
View session
Deploy a Kubernetes-native ValidatingAdmissionPolicy with CEL rules to both service and management clusters that restricts pod image pulls to Microsoft-controlled registries only (SVC ACR, OCP ACR, MCR, and kubernetesshared.azurecr.io). External public registries (quay.io, registry.k8s.io, registry.redhat.io) are excluded since all images from those sources are mirrored to ACR via ImageMirror pipeline steps or pull-through cache rules. The policy uses fail-closed semantics (failurePolicy: Fail, parameterNotFoundAction: Deny) with system namespace exclusions. Includes E2E tests verifying policy presence and enforcement. JIRA: ARO-22152
- Remove unused excludedNamespaces from values.yaml — the system namespace exclusions are hardcoded in the CEL matchConditions expression and cannot be driven from the ConfigMap - Change deny test label from Positive to Negative to match repo convention for expected-rejection tests
- Reorder imports in image_registry_policy.go to match gci sections (Azure SDK imports after k8s.io imports, not between dot and k8s groups) - Add new Image Registry Policy E2E test names to suite fixture files (integration_parallel, stage_parallel, prod_parallel) - Update nonlocal-e2e-specs.txt with new test specs
Apply yamlfmt-required formatting to CEL expressions in the ValidatingAdmissionPolicy template. Use GenerateName instead of hardcoded namespace names in E2E tests to prevent race conditions when tests run in parallel.
Convert multi-line >- folded scalar blocks to single-line quoted strings in the ValidatingAdmissionPolicy. yamlfmt adds an extra blank line after >- blocks on each run, making verify-yamlfmt fail. Single-line quoted strings are stable across yamlfmt passes.
Match the retry configuration used by all other Helm steps in both svc-pipeline.yaml and mgmt-pipeline.yaml.
Dev environments use containerized databases and observability tools (postgres, jaeger, grafana, otel) that pull from docker.io and related registries. These are not used in int/stg/prod which use Azure managed services. Add imageRegistryPolicy.extraAllowedRegistries config field with dev-only overrides so the ValidatingAdmissionPolicy allows these images in dev while keeping int/stg/prod locked down to ACR+MCR only. Move the templatize values template to image-registry-policy/values.yaml (outside the Helm chart directory) to use Go template range syntax for dynamic registry list expansion without conflicting with Helm's chart loader which requires valid YAML in deploy/values.yaml.
docker.io is now in the dev environment allowlist for dev tooling, so the negative E2E test must use a registry that is disallowed in all environments.
Default to Audit in all environments so the policy can be safely deployed everywhere. Override to Deny in dev where image exceptions are already in place. This enables a phased approach: deploy with Audit in int/stg/prod, review audit logs for violations, then switch to Deny once all images are verified. E2E deny test marked DevelopmentOnly since higher envs start in Audit mode. Policy-exists test relaxed to accept any validationAction.
- Remove namespace.yaml template; rely on releaseNamespace in pipeline step for namespace creation (matching acrpull pattern) - Use .Release.Namespace instead of .Values.namespace in templates - Remove redundant E2E tests: VAP-exists test (tests installation not effect) and allowed-registries test (implicitly tested by env provisioning) - Add image-registry-policy as dependsOn for all helm steps on both mgmt and svc pipelines so policy is active before other deployments
Add image-registry-policy template files to yamllint ignore list since they use Go template range blocks that break YAML parsing. Regenerate test suite fixtures to correctly place DevelopmentOnly test in the rp-api-compat-all/parallel (development) suite only.
Previous e2e-parallel failure was caused by systemic Prow dev environment issues (cluster creation timeouts across all tests), not by this PR's changes. Rebasing to pick up 69 new commits from main and proactively prevent post-merge conflicts.
- Add pods/ephemeralcontainers subresource to resourceRules so the policy also covers ephemeral container additions via the subresource API path - Remove unused namespace field from values.yaml template - Add upgrade-aks-cluster dependency to image-registry-policy Helm steps in both svc-pipeline and mgmt-pipeline to avoid applying the policy against an older API surface during AKS upgrades
Anthony Vollmer (avollmer-redhat)
force-pushed
the
fsi-image-registry-policy-clean
branch
from
May 5, 2026 13:28
06393b1 to
44611eb
Compare
Collaborator
Author
|
/test e2e-parallel |
|
Anthony Vollmer (@avollmer-redhat): The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
|
PR needs rebase. DetailsInstructions 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. |
Collaborator
Author
|
Closing — this clean-baseline PR is now obsolete. Context: PR #4690 was merged, reverted, and then re-landed with fixes via #5367 (May 24). The image-registry-policy VAP is now live on main with pipeline integration, e2e tests, and CPO registry exceptions. This PR's original purpose was to prove the VAP works without registry exceptions. That goal is now better served by a small, fresh PR against current main once the FSI compliance chain completes:
A new PR will be ~50 lines vs rebasing 14 commits across 3,127 diverged commits with 23+ conflict blocks. |
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.
Purpose
Clean baseline clone of #4690 without Rael's
d858c0becommit that addedquay.io,registry.k8s.io, andregistry.redhat.ioas registry exceptions.This PR exists to run e2e-parallel against the original VAP implementation and prove the policy does not block any legitimate workloads. The management cluster infrastructure saturation that caused 77 consecutive failures has resolved (see AROSLSRE-768).
What this proves
quay.io,registry.k8s.io,registry.redhat.ioaddedContext
quay.io,registry.k8s.io,registry.redhat.io) are ORAS-mirrored to ACR before deploymentDo not merge
This PR is for CI validation only. The canonical PR is #4690 (after reverting the registry exceptions commit).