Skip to content

OCPBUGS-43353: Add pattern validation for registry entries in image config #2787

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.

Sign up for GitHub

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

Open
reedcort wants to merge 1 commit into
base: master
Choose a base branch
from
Open

OCPBUGS-43353: Add pattern validation for registry entries in image config #2787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
304 changes: 304 additions & 0 deletions config/v1/tests/images.config.openshift.io/AAA_ungated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,126 @@ tests:
apiVersion: config.openshift.io/v1
kind: Image
spec: {}
- name: Should not allow creating an Image with a tagged registry entry in blockedRegistries
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
blockedRegistries: ["registry.io/myrepo:latest"]
expectedError: "spec.registrySources.blockedRegistries[0]"
- name: Should not allow creating an Image with a tagged registry entry in allowedRegistries
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
allowedRegistries: ["registry.io/myrepo:latest"]
expectedError: "spec.registrySources.allowedRegistries[0]"
- name: Should not allow creating an Image with a tagged registry entry in insecureRegistries
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
insecureRegistries: ["registry.io/myrepo:latest"]
expectedError: "spec.registrySources.insecureRegistries[0]"
- name: Should not allow creating an Image with a digest registry entry in blockedRegistries
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
blockedRegistries: ["registry.io/myrepo@sha256:abc123"]
expectedError: "spec.registrySources.blockedRegistries[0]"
- name: Should not allow creating an Image with a digest registry entry in allowedRegistries
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
allowedRegistries: ["registry.io/myrepo@sha256:abc123"]
expectedError: "spec.registrySources.allowedRegistries[0]"
- name: Should not allow creating an Image with a digest registry entry in insecureRegistries
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
insecureRegistries: ["registry.io/myrepo@sha256:abc123"]
expectedError: "spec.registrySources.insecureRegistries[0]"
- name: Should not allow creating an Image with an empty string in blockedRegistries
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
blockedRegistries: [""]
expectedError: "spec.registrySources.blockedRegistries[0]"
- name: Should not allow creating an Image with an empty string in allowedRegistries
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
allowedRegistries: [""]
expectedError: "spec.registrySources.allowedRegistries[0]"
- name: Should not allow creating an Image with an empty string in insecureRegistries
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
insecureRegistries: [""]
expectedError: "spec.registrySources.insecureRegistries[0]"
- name: Should allow creating an Image with valid registry entries in blockedRegistries and insecureRegistries
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
blockedRegistries:
- "registry.io"
- "registry.io/myrepo"
- "*.example.com"
- "localhost:5000/repo"
insecureRegistries:
- "insecure.registry.io"
- "*.insecure.example.com"
expected: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
blockedRegistries:
- "registry.io"
- "registry.io/myrepo"
- "*.example.com"
- "localhost:5000/repo"
insecureRegistries:
- "insecure.registry.io"
- "*.insecure.example.com"
- name: Should allow creating an Image with valid registry entries in allowedRegistries
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
allowedRegistries:
- "registry.io"
- "registry.io/myrepo"
- "*.example.com"
- "localhost:5000/repo"
expected: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
allowedRegistries:
- "registry.io"
- "registry.io/myrepo"
- "*.example.com"
- "localhost:5000/repo"
onUpdate:
- name: Should allow updating other fields with an invalid persisted registrySources in spec
initialCRDPatches:
Expand Down Expand Up @@ -114,3 +234,187 @@ tests:
allowedRegistries: ["test"]
blockedRegistries: ["test"]
expectedError: 'Only one of blockedRegistries or allowedRegistries may be set'
- name: Should allow updating other fields when an invalid tagged blockedRegistries entry is persisted
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/registrySources/properties/blockedRegistries/items/x-kubernetes-validations
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
blockedRegistries: ["registry.io/myrepo:latest"]
updated: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
externalRegistryHostnames: ["registry.example.com"]
registrySources:
blockedRegistries: ["registry.io/myrepo:latest"]
expected: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
externalRegistryHostnames: ["registry.example.com"]
registrySources:
blockedRegistries: ["registry.io/myrepo:latest"]
- name: Should allow updating other fields when an invalid tagged allowedRegistries entry is persisted
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/registrySources/properties/allowedRegistries/items/x-kubernetes-validations
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
allowedRegistries: ["registry.io/myrepo:latest"]
updated: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
externalRegistryHostnames: ["registry.example.com"]
registrySources:
allowedRegistries: ["registry.io/myrepo:latest"]
expected: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
externalRegistryHostnames: ["registry.example.com"]
registrySources:
allowedRegistries: ["registry.io/myrepo:latest"]
- name: Should allow updating other fields when an invalid tagged insecureRegistries entry is persisted
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/registrySources/properties/insecureRegistries/items/x-kubernetes-validations
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
insecureRegistries: ["registry.io/myrepo:latest"]
updated: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
externalRegistryHostnames: ["registry.example.com"]
registrySources:
insecureRegistries: ["registry.io/myrepo:latest"]
expected: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
externalRegistryHostnames: ["registry.example.com"]
registrySources:
insecureRegistries: ["registry.io/myrepo:latest"]
- name: Should not allow adding a new invalid tagged entry to blockedRegistries even when an invalid entry is already persisted
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/registrySources/properties/blockedRegistries/items/x-kubernetes-validations
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
blockedRegistries: ["registry.io/myrepo:latest"]
updated: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
blockedRegistries: ["registry.io/myrepo:latest", "other.com/repo:v1"]
expectedError: "spec.registrySources.blockedRegistries"
- name: Should not allow appending to blockedRegistries when an invalid entry is already persisted because listType atomic re-validates the whole list
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/registrySources/properties/blockedRegistries/items/x-kubernetes-validations
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
blockedRegistries: ["registry.io/myrepo:latest"]
updated: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
blockedRegistries: ["registry.io/myrepo:latest", "valid.registry.io"]
expectedError: "spec.registrySources.blockedRegistries"
- name: Should allow updating other fields when an empty string is persisted in blockedRegistries
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/registrySources/properties/blockedRegistries/items/minLength
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/registrySources/properties/blockedRegistries/items/x-kubernetes-validations
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
blockedRegistries: [""]
updated: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
externalRegistryHostnames: ["registry.example.com"]
registrySources:
blockedRegistries: [""]
expected: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
externalRegistryHostnames: ["registry.example.com"]
registrySources:
blockedRegistries: [""]
- name: Should allow updating other fields when an empty string is persisted in allowedRegistries
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/registrySources/properties/allowedRegistries/items/minLength
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/registrySources/properties/allowedRegistries/items/x-kubernetes-validations
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
allowedRegistries: [""]
updated: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
externalRegistryHostnames: ["registry.example.com"]
registrySources:
allowedRegistries: [""]
expected: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
externalRegistryHostnames: ["registry.example.com"]
registrySources:
allowedRegistries: [""]
- name: Should allow updating other fields when an empty string is persisted in insecureRegistries
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/registrySources/properties/insecureRegistries/items/minLength
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/registrySources/properties/insecureRegistries/items/x-kubernetes-validations
initial: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
registrySources:
insecureRegistries: [""]
updated: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
externalRegistryHostnames: ["registry.example.com"]
registrySources:
insecureRegistries: [""]
expected: |
apiVersion: config.openshift.io/v1
kind: Image
spec:
externalRegistryHostnames: ["registry.example.com"]
registrySources:
insecureRegistries: [""]
30 changes: 30 additions & 0 deletions config/v1/types_image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,20 +165,50 @@ type RegistryLocation struct {
// +kubebuilder:validation:XValidation:rule="has(self.blockedRegistries) ? !has(self.allowedRegistries) : true",message="Only one of blockedRegistries or allowedRegistries may be set"
type RegistrySources struct {
// insecureRegistries are registries which do not have a valid TLS certificates or only support HTTP connections.
// Each entry must be a valid registry scope in the format hostname[:port][/path],
// optionally prefixed with "*." for wildcard subdomains (e.g., "*.example.com").
// The hostname must consist of valid DNS labels separated by dots, where each label
// contains only alphanumeric characters and hyphens and does not start or end with a hyphen.
// Entries must not be empty, must not include tags (e.g., ":latest") or digests (e.g., "@sha256:..."),
// and must be at most 512 characters in length. The list may contain at most 128 entries.
// +optional
// +listType=atomic
// +kubebuilder:validation:MaxItems=128
// +kubebuilder:validation:items:MinLength=1
// +kubebuilder:validation:items:MaxLength=512
// +kubebuilder:validation:items:XValidation:rule="self.matches('^\\\\*(?:\\\\.(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+$|^((?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])(?:(?:\\\\.(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+)?(?::[0-9]+)?)(?:(?:/[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?)+)?$')",message="each registry must be a valid hostname[:port][/path] or wildcard *.hostname format without tags or digests"
Comment on lines +168 to +179
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Wildcard entries currently reject documented :port and /path scopes.

The regex splits wildcard hosts into a separate ^\\*...$ branch, so *.example.com/ns and *.example.com:5000/ns fail validation even though the updated godoc says the format is hostname[:port][/path] with an optional *. prefix. Please fold the wildcard prefix into the main hostname alternative, or narrow the docs/message if wildcarded repo scopes are intentionally unsupported.

Also applies to: 182-195, 198-211

Copy link
Copy Markdown
Contributor

@everettraven everettraven Apr 16, 2026

Choose a reason for hiding this comment

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

This seems like a good find, although I will note that I haven't personally verified the regex myself.

@reedcort do we intend for something like the examples above to be valid wildcards or only something like *.example.com?

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 16, 2026

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

InsecureRegistries []string `json:"insecureRegistries,omitempty"`
Comment thread
everettraven marked this conversation as resolved.
// blockedRegistries cannot be used for image pull and push actions. All other registries are permitted.
// Each entry must be a valid registry scope in the format hostname[:port][/path],
// optionally prefixed with "*." for wildcard subdomains (e.g., "*.example.com").
// The hostname must consist of valid DNS labels separated by dots, where each label
// contains only alphanumeric characters and hyphens and does not start or end with a hyphen.
// Entries must not be empty, must not include tags (e.g., ":latest") or digests (e.g., "@sha256:..."),
// and must be at most 512 characters in length. The list may contain at most 128 entries.
//
// Only one of BlockedRegistries or AllowedRegistries may be set.
// +optional
// +listType=atomic
// +kubebuilder:validation:MaxItems=128
// +kubebuilder:validation:items:MinLength=1
// +kubebuilder:validation:items:MaxLength=512
// +kubebuilder:validation:items:XValidation:rule="self.matches('^\\\\*(?:\\\\.(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+$|^((?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])(?:(?:\\\\.(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+)?(?::[0-9]+)?)(?:(?:/[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?)+)?$')",message="each registry must be a valid hostname[:port][/path] or wildcard *.hostname format without tags or digests"
BlockedRegistries []string `json:"blockedRegistries,omitempty"`
// allowedRegistries are the only registries permitted for image pull and push actions. All other registries are denied.
// Each entry must be a valid registry scope in the format hostname[:port][/path],
// optionally prefixed with "*." for wildcard subdomains (e.g., "*.example.com").
// The hostname must consist of valid DNS labels separated by dots, where each label
// contains only alphanumeric characters and hyphens and does not start or end with a hyphen.
// Entries must not be empty, must not include tags (e.g., ":latest") or digests (e.g., "@sha256:..."),
// and must be at most 512 characters in length. The list may contain at most 128 entries.
//
// Only one of BlockedRegistries or AllowedRegistries may be set.
// +optional
// +listType=atomic
// +kubebuilder:validation:MaxItems=128
// +kubebuilder:validation:items:MinLength=1
// +kubebuilder:validation:items:MaxLength=512
// +kubebuilder:validation:items:XValidation:rule="self.matches('^\\\\*(?:\\\\.(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+$|^((?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])(?:(?:\\\\.(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+)?(?::[0-9]+)?)(?:(?:/[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?)+)?$')",message="each registry must be a valid hostname[:port][/path] or wildcard *.hostname format without tags or digests"
AllowedRegistries []string `json:"allowedRegistries,omitempty"`
Comment thread
everettraven marked this conversation as resolved.
// containerRuntimeSearchRegistries are registries that will be searched when pulling images that do not have fully qualified
// domains in their pull specs. Registries will be searched in the order provided in the list.
Expand Down
Loading