From cf48c6059fccd96845e52ff4a5e6185e2548fece Mon Sep 17 00:00:00 2001 From: Qi Wang Date: Wed, 27 Mar 2024 10:30:31 -0400 Subject: [PATCH] Support Cluster ImagePolicy CRD --- cmd/machine-config-controller/start.go | 1 + docs/ClusterImagePolicyDesign.md | 98 +++++ hack/crds-sync.sh | 2 +- ...rimagepolicy-TechPreviewNoUpgrade.crd.yaml | 398 ++++++++++++++++++ .../machineconfigcontroller/clusterrole.yaml | 4 +- pkg/apihelpers/apihelpers.go | 10 + pkg/controller/bootstrap/bootstrap.go | 37 +- .../testdata/bootstrap/featuregate.yaml | 2 + .../container_runtime_config_bootstrap.go | 3 +- ...container_runtime_config_bootstrap_test.go | 2 +- .../container_runtime_config_controller.go | 255 ++++++++--- ...ontainer_runtime_config_controller_test.go | 205 +++++++-- .../container-runtime-config/helpers.go | 168 +++++++- .../container-runtime-config/helpers_test.go | 341 ++++++++++++++- pkg/daemon/constants/constants.go | 3 + pkg/daemon/update.go | 5 + test/e2e-bootstrap/bootstrap_test.go | 5 +- 17 files changed, 1409 insertions(+), 130 deletions(-) create mode 100644 docs/ClusterImagePolicyDesign.md create mode 100644 install/0000_10_config-operator_01_clusterimagepolicy-TechPreviewNoUpgrade.crd.yaml diff --git a/cmd/machine-config-controller/start.go b/cmd/machine-config-controller/start.go index f417ed7830..9492707db8 100644 --- a/cmd/machine-config-controller/start.go +++ b/cmd/machine-config-controller/start.go @@ -171,6 +171,7 @@ func createControllers(ctx *ctrlcommon.ControllerContext) []ctrlcommon.Controlle ctx.ConfigInformerFactory.Config().V1().Images(), ctx.ConfigInformerFactory.Config().V1().ImageDigestMirrorSets(), ctx.ConfigInformerFactory.Config().V1().ImageTagMirrorSets(), + ctx.ConfigInformerFactory, ctx.OperatorInformerFactory.Operator().V1alpha1().ImageContentSourcePolicies(), ctx.ConfigInformerFactory.Config().V1().ClusterVersions(), ctx.ClientBuilder.KubeClientOrDie("container-runtime-config-controller"), diff --git a/docs/ClusterImagePolicyDesign.md b/docs/ClusterImagePolicyDesign.md new file mode 100644 index 0000000000..6d97324adb --- /dev/null +++ b/docs/ClusterImagePolicyDesign.md @@ -0,0 +1,98 @@ +## Summary +ClusterImagePolicy CRD is managed by ContainerRuntimeConfig controller. This CRD allows setting up configurations for CRI-O to verify the container images signed using [Sigstore](https://www.sigstore.dev/) tools. + +## Goals +Generating corresponding CRI-O configuration files for image signature verification. Rollout ClusterImagePolicy to `/etc/containers/policy.json` for cluster wide configuration. Roll out the registries configuration to `/etc/containers/registries.d/sigstore-registries.yaml`. + +## Non-Goals +Rolling out configuration for OCP payload repositories. The (super scope of) OCP payload repositories will not be written to the configuration files. + +## CRD +[ClusterImagePolicy CRD](https://github.com/openshift/api/blob/master/config/v1alpha1/0000_10_config-operator_01_clusterimagepolicy-TechPreviewNoUpgrade.crd.yaml) + +## Example + +Below is an example of a ClusterImagePolicy CRD. + +```yaml +apiVersion: config.openshift.io/v1alpha1 +kind: ClusterImagePolicy +metadata: + name: p0 +spec: + scopes: + - registry.ci.openshift.org + - example.com/global + policy: + rootOfTrust: + policyType: PublicKey + publicKey: + keyData: Zm9vIGJhcg== + signedIdentity: + matchPolicy: MatchRepoDigestOrExact +``` + +Save the above clusterimagepolicy locally, for example as pubKeyPolicy.yaml. +Now apply the clusterimagepolicy that you created: + +```shell +oc apply -f pubKeyPolicy.yaml +``` + +Check that it was created: + +```shell +oc get clusterimagepolicy +NAME AGE +p0 9s + +``` + +## Validation and Troubleshooting +The status.conditions.message field shows if the CR has conflicting scope(s). The below ClusterImagePolicy is in pending status because the scope for OCP release payload is skipped. + +```shell +# the clusterimagepolicy has scope registry.build05.ci.openshift.org that is the OCP release payload repository +$ oc describe clusterimagepolicy.config.openshift.io/p0 +... +... +Status: + Conditions: + Last Transition Time: 2024-02-04T03:24:36Z + Message: has conflict scope(s) ["registry.build05.ci.openshift.org"] of Openshift payload repository registry.build05.ci.openshift.org/ci-ln-gfmf0yb/release, skip the scope(s) + Observed Generation: 1 + Reason: ConflictScopes + Status: True + Type: Pending +``` + +The machine-config-controller logs will show the error message if the ClusterImagePolicy and Image CR has conflicting configurations. Controller will fail to roll out the CR. +- if blocked registries configured by Image CR exist, the clusterimagepolicy scopes must not equal to or nested under blockedRegistries +- if allowed registries configured by Image CR exist, the clusterimagepolicy scopes nested under the allowedRegistries +For example, the below error message is shown when the ClusterImagePolicy and blockedRegistries of Image CR has conflicting configurations. + +```shell +I0204 03:51:18.253145 1 container_runtime_config_controller.go:497] Error syncing image config openshift-config: could not Create/Update MachineConfig: could not update policy json with new changes: clusterimagePolicy configured for the scope example.com/global is nested inside blockedRegistries ``` +``` + +## Implementation Details +The ContainerRuntimeConfigController would perform the following steps: + +1. Validate the ClusterImagePolicy objects are not for OCP release payload repositories. + +2. Render the current MachineConfigs (storage.files.contents[policy.json]) into the originalPolicyIgn + +3. Serialize the cluster level policies to `policy.json`. + +4. Add registries configuration to `sigstore-registries.yaml`. This configuration is used to specify the sigstore is being used as the image signature verification backend. + +5. Update the ignition file `/etc/containers/policy.json` within the `99--generated-registries` MachineConfig. + +6. Create or Update the ignition file `/etc/containers/registries.d/sigstore-registries.yaml` within the `99--generated-imagepolicies` MachineConfig. + +After deletion all of the ClusterImagePolicy instance the config will be reverted to the original policy.json. + +## See Also +see **[containers-policy.json(5)](https://github.com/containers/image/blob/main/docs/containers-policy.json.5.md)**, **[containers-registries.d(5)](https://github.com/containers/image/blob/main/docs/containers-registries.d.5.md)** for more information. + + diff --git a/hack/crds-sync.sh b/hack/crds-sync.sh index aee6e844e6..76abb7eca7 100755 --- a/hack/crds-sync.sh +++ b/hack/crds-sync.sh @@ -17,7 +17,7 @@ for crd in "${CRDS_MAPPING[@]}" ; do done #this one goes in manifests rather than install, but should it? +cp "vendor/github.com/openshift/api/config/v1alpha1/0000_10_config-operator_01_clusterimagepolicy-TechPreviewNoUpgrade.crd.yaml" "install/0000_10_config-operator_01_clusterimagepolicy-TechPreviewNoUpgrade.crd.yaml" cp "vendor/github.com/openshift/api/machineconfiguration/v1/0000_80_controllerconfig.crd.yaml" "manifests/controllerconfig.crd.yaml" cp "vendor/github.com/openshift/api/machineconfiguration/v1alpha1/0000_80_machineconfignode-TechPreviewNoUpgrade.crd.yaml" "manifests/0000_80_machine-config-operator_01_machineconfignode-TechPreviewNoUpgrade.crd.yaml" cp "vendor/github.com/openshift/api/operator/v1/0000_80_machine-config-operator_01_config.crd.yaml" "install/0000_80_machine-config-operator_01_config.crd.yaml" - diff --git a/install/0000_10_config-operator_01_clusterimagepolicy-TechPreviewNoUpgrade.crd.yaml b/install/0000_10_config-operator_01_clusterimagepolicy-TechPreviewNoUpgrade.crd.yaml new file mode 100644 index 0000000000..834c03ae11 --- /dev/null +++ b/install/0000_10_config-operator_01_clusterimagepolicy-TechPreviewNoUpgrade.crd.yaml @@ -0,0 +1,398 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + api-approved.openshift.io: https://github.com/openshift/api/pull/1457 + include.release.openshift.io/ibm-cloud-managed: "true" + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + release.openshift.io/feature-set: TechPreviewNoUpgrade + name: clusterimagepolicies.config.openshift.io +spec: + group: config.openshift.io + names: + kind: ClusterImagePolicy + listKind: ClusterImagePolicyList + plural: clusterimagepolicies + singular: clusterimagepolicy + scope: Cluster + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: "ClusterImagePolicy holds cluster-wide configuration for image + signature verification \n Compatibility level 4: No compatibility is provided, + the API can change at any point for any reason. These capabilities should + not be used by applications needing long term support." + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: spec contains the configuration for the cluster image policy. + properties: + policy: + description: policy contains configuration to allow scopes to be verified, + and defines how images not matching the verification policy will + be treated. + properties: + rootOfTrust: + description: rootOfTrust specifies the root of trust for the policy. + properties: + fulcioCAWithRekor: + description: 'fulcioCAWithRekor defines the root of trust + based on the Fulcio certificate and the Rekor public key. + For more information about Fulcio and Rekor, please refer + to the document at: https://github.com/sigstore/fulcio and + https://github.com/sigstore/rekor' + properties: + fulcioCAData: + description: fulcioCAData contains inline base64-encoded + data for the PEM format fulcio CA. fulcioCAData must + be at most 8192 characters. + format: byte + maxLength: 8192 + type: string + fulcioSubject: + description: fulcioSubject specifies OIDC issuer and the + email of the Fulcio authentication configuration. + properties: + oidcIssuer: + description: 'oidcIssuer contains the expected OIDC + issuer. It will be verified that the Fulcio-issued + certificate contains a (Fulcio-defined) certificate + extension pointing at this OIDC issuer URL. When + Fulcio issues certificates, it includes a value + based on an URL inside the client-provided ID token. + Example: "https://expected.OIDC.issuer/"' + type: string + x-kubernetes-validations: + - message: oidcIssuer must be a valid URL + rule: isURL(self) + signedEmail: + description: 'signedEmail holds the email address + the the Fulcio certificate is issued for. Example: + "expected-signing-user@example.com"' + type: string + x-kubernetes-validations: + - message: invalid email address + rule: self.matches('^\\S+@\\S+$') + required: + - oidcIssuer + - signedEmail + type: object + rekorKeyData: + description: rekorKeyData contains inline base64-encoded + data for the PEM format from the Rekor public key. rekorKeyData + must be at most 8192 characters. + format: byte + maxLength: 8192 + type: string + required: + - fulcioCAData + - fulcioSubject + - rekorKeyData + type: object + policyType: + description: policyType serves as the union's discriminator. + Users are required to assign a value to this field, choosing + one of the policy types that define the root of trust. "PublicKey" + indicates that the policy relies on a sigstore publicKey + and may optionally use a Rekor verification. "FulcioCAWithRekor" + indicates that the policy is based on the Fulcio certification + and incorporates a Rekor verification. + enum: + - PublicKey + - FulcioCAWithRekor + type: string + publicKey: + description: publicKey defines the root of trust based on + a sigstore public key. + properties: + keyData: + description: keyData contains inline base64-encoded data + for the PEM format public key. KeyData must be at most + 8192 characters. + format: byte + maxLength: 8192 + type: string + rekorKeyData: + description: rekorKeyData contains inline base64-encoded + data for the PEM format from the Rekor public key. rekorKeyData + must be at most 8192 characters. + format: byte + maxLength: 8192 + type: string + required: + - keyData + type: object + required: + - policyType + type: object + x-kubernetes-validations: + - message: publicKey is required when policyType is PublicKey, + and forbidden otherwise + rule: 'has(self.policyType) && self.policyType == ''PublicKey'' + ? has(self.publicKey) : !has(self.publicKey)' + - message: fulcioCAWithRekor is required when policyType is FulcioCAWithRekor, + and forbidden otherwise + rule: 'has(self.policyType) && self.policyType == ''FulcioCAWithRekor'' + ? has(self.fulcioCAWithRekor) : !has(self.fulcioCAWithRekor)' + signedIdentity: + description: signedIdentity specifies what image identity the + signature claims about the image. The required matchPolicy field + specifies the approach used in the verification process to verify + the identity in the signature and the actual image identity, + the default matchPolicy is "MatchRepoDigestOrExact". + properties: + exactRepository: + description: exactRepository is required if matchPolicy is + set to "ExactRepository". + properties: + repository: + description: repository is the reference of the image + identity to be matched. The value should be a repository + name (by omitting the tag or digest) in a registry implementing + the "Docker Registry HTTP API V2". For example, docker.io/library/busybox + maxLength: 512 + type: string + x-kubernetes-validations: + - message: invalid repository or prefix in the signedIdentity, + should not include the tag or digest + rule: 'self.matches(''.*:([\\w][\\w.-]{0,127})$'')? + self.matches(''^(localhost:[0-9]+)$''): true' + - message: invalid repository or prefix in the signedIdentity + 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]))+(?::[0-9]+)?)|(localhost(?::[0-9]+)?))(?:(?:/[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?)+)?$') + required: + - repository + type: object + matchPolicy: + description: matchPolicy sets the type of matching to be used. + Valid values are "MatchRepoDigestOrExact", "MatchRepository", + "ExactRepository", "RemapIdentity". When omitted, the default + value is "MatchRepoDigestOrExact". If set matchPolicy to + ExactRepository, then the exactRepository must be specified. + If set matchPolicy to RemapIdentity, then the remapIdentity + must be specified. "MatchRepoDigestOrExact" means that the + identity in the signature must be in the same repository + as the image identity if the image identity is referenced + by a digest. Otherwise, the identity in the signature must + be the same as the image identity. "MatchRepository" means + that the identity in the signature must be in the same repository + as the image identity. "ExactRepository" means that the + identity in the signature must be in the same repository + as a specific identity specified by "repository". "RemapIdentity" + means that the signature must be in the same as the remapped + image identity. Remapped image identity is obtained by replacing + the "prefix" with the specified “signedPrefix” if the the + image identity matches the specified remapPrefix. + enum: + - MatchRepoDigestOrExact + - MatchRepository + - ExactRepository + - RemapIdentity + type: string + remapIdentity: + description: remapIdentity is required if matchPolicy is set + to "RemapIdentity". + properties: + prefix: + description: prefix is the prefix of the image identity + to be matched. If the image identity matches the specified + prefix, that prefix is replaced by the specified “signedPrefix” + (otherwise it is used as unchanged and no remapping + takes place). This useful when verifying signatures + for a mirror of some other repository namespace that + preserves the vendor’s repository structure. The prefix + and signedPrefix values can be either host[:port] values + (matching exactly the same host[:port], string), repository + namespaces, or repositories (i.e. they must not contain + tags/digests), and match as prefixes of the fully expanded + form. For example, docker.io/library/busybox (not busybox) + to specify that single repository, or docker.io/library + (not an empty string) to specify the parent namespace + of docker.io/library/busybox. + maxLength: 512 + type: string + x-kubernetes-validations: + - message: invalid repository or prefix in the signedIdentity, + should not include the tag or digest + rule: 'self.matches(''.*:([\\w][\\w.-]{0,127})$'')? + self.matches(''^(localhost:[0-9]+)$''): true' + - message: invalid repository or prefix in the signedIdentity + 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]))+(?::[0-9]+)?)|(localhost(?::[0-9]+)?))(?:(?:/[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?)+)?$') + signedPrefix: + description: signedPrefix is the prefix of the image identity + to be matched in the signature. The format is the same + as "prefix". The values can be either host[:port] values + (matching exactly the same host[:port], string), repository + namespaces, or repositories (i.e. they must not contain + tags/digests), and match as prefixes of the fully expanded + form. For example, docker.io/library/busybox (not busybox) + to specify that single repository, or docker.io/library + (not an empty string) to specify the parent namespace + of docker.io/library/busybox. + maxLength: 512 + type: string + x-kubernetes-validations: + - message: invalid repository or prefix in the signedIdentity, + should not include the tag or digest + rule: 'self.matches(''.*:([\\w][\\w.-]{0,127})$'')? + self.matches(''^(localhost:[0-9]+)$''): true' + - message: invalid repository or prefix in the signedIdentity + 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]))+(?::[0-9]+)?)|(localhost(?::[0-9]+)?))(?:(?:/[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?)+)?$') + required: + - prefix + - signedPrefix + type: object + required: + - matchPolicy + type: object + x-kubernetes-validations: + - message: exactRepository is required when matchPolicy is ExactRepository, + and forbidden otherwise + rule: '(has(self.matchPolicy) && self.matchPolicy == ''ExactRepository'') + ? has(self.exactRepository) : !has(self.exactRepository)' + - message: remapIdentity is required when matchPolicy is RemapIdentity, + and forbidden otherwise + rule: '(has(self.matchPolicy) && self.matchPolicy == ''RemapIdentity'') + ? has(self.remapIdentity) : !has(self.remapIdentity)' + required: + - rootOfTrust + type: object + scopes: + description: 'scopes defines the list of image identities assigned + to a policy. Each item refers to a scope in a registry implementing + the "Docker Registry HTTP API V2". Scopes matching individual images + are named Docker references in the fully expanded form, either using + a tag or digest. For example, docker.io/library/busybox:latest (not + busybox:latest). More general scopes are prefixes of individual-image + scopes, and specify a repository (by omitting the tag or digest), + a repository namespace, or a registry host (by only specifying the + host name and possibly a port number) or a wildcard expression starting + with `*.`, for matching all subdomains (not including a port number). + Wildcards are only supported for subdomain matching, and may not + be used in the middle of the host, i.e. *.example.com is a valid + case, but example*.*.com is not. Please be aware that the scopes + should not be nested under the repositories of OpenShift Container + Platform images. If configured, the policies for OpenShift Container + Platform repositories will not be in effect. For additional details + about the format, please refer to the document explaining the docker + transport field, which can be found at: https://github.com/containers/image/blob/main/docs/containers-policy.json.5.md#docker' + items: + maxLength: 512 + type: string + x-kubernetes-validations: + - message: invalid image scope format, scope must contain a fully + qualified domain name or 'localhost' + rule: 'size(self.split(''/'')[0].split(''.'')) == 1 ? self.split(''/'')[0].split(''.'')[0].split('':'')[0] + == ''localhost'' : true' + - message: invalid image scope with wildcard, a wildcard can only + be at the start of the domain and is only supported for subdomain + matching, not path matching + rule: 'self.contains(''*'') ? self.matches(''^\\*(?:\\.(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+$'') + : true' + - message: invalid repository namespace or image specification in + the image scope + rule: '!self.contains(''*'') ? 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]))+(?::[0-9]+)?)|(localhost(?::[0-9]+)?))(?:(?:/[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?)+)?)(?::([\\w][\\w.-]{0,127}))?(?:@([A-Za-z][A-Za-z0-9]*(?:[-_+.][A-Za-z][A-Za-z0-9]*)*[:][[:xdigit:]]{32,}))?$'') + : true' + maxItems: 256 + type: array + x-kubernetes-list-type: set + required: + - policy + - scopes + type: object + status: + description: status contains the observed state of the resource. + properties: + conditions: + description: conditions provide details on the status of this API + Resource. + items: + description: "Condition contains details for one aspect of the current + state of this API Resource. --- This struct is intended for direct + use as an array at the field path .status.conditions. For example, + \n type FooStatus struct{ // Represents the observations of a + foo's current state. // Known .status.conditions.type are: \"Available\", + \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge + // +listType=map // +listMapKey=type Conditions []metav1.Condition + `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" + protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }" + properties: + lastTransitionTime: + description: lastTransitionTime is the last time the condition + transitioned from one status to another. This should be when + the underlying condition changed. If that is not known, then + using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: message is a human readable message indicating + details about the transition. This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: observedGeneration represents the .metadata.generation + that the condition was set based upon. For instance, if .metadata.generation + is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the current + state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: reason contains a programmatic identifier indicating + the reason for the condition's last transition. Producers + of specific condition types may define expected values and + meanings for this field, and whether the values are considered + a guaranteed API. The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + --- Many .condition.type values are consistent across resources + like Available, but because arbitrary conditions can be useful + (see .node.status.conditions), the ability to deconflict is + important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + type: object + required: + - spec + type: object + served: true + storage: true + subresources: + status: {} diff --git a/manifests/machineconfigcontroller/clusterrole.yaml b/manifests/machineconfigcontroller/clusterrole.yaml index 7578e1f187..fff1f573b5 100644 --- a/manifests/machineconfigcontroller/clusterrole.yaml +++ b/manifests/machineconfigcontroller/clusterrole.yaml @@ -13,10 +13,10 @@ rules: resources: ["configmaps", "secrets"] verbs: ["*"] - apiGroups: ["config.openshift.io"] - resources: ["images", "clusterversions", "featuregates", "nodes", "nodes/status"] + resources: ["images", "clusterversions", "featuregates", "nodes", "nodes/status", "clusterimagepolicies/status"] verbs: ["*"] - apiGroups: ["config.openshift.io"] - resources: ["schedulers", "apiservers", "infrastructures", "imagedigestmirrorsets", "imagetagmirrorsets"] + resources: ["schedulers", "apiservers", "infrastructures", "imagedigestmirrorsets", "imagetagmirrorsets", "clusterimagepolicies"] verbs: ["get", "list", "watch"] - apiGroups: ["operator.openshift.io"] resources: ["imagecontentsourcepolicies"] diff --git a/pkg/apihelpers/apihelpers.go b/pkg/apihelpers/apihelpers.go index e8f41d2bd1..70caaedaf7 100644 --- a/pkg/apihelpers/apihelpers.go +++ b/pkg/apihelpers/apihelpers.go @@ -99,6 +99,16 @@ func NewKubeletConfigCondition(condType mcfgv1.KubeletConfigStatusConditionType, } } +func NewCondition(condType string, status metav1.ConditionStatus, reason, message string) *metav1.Condition { + return &metav1.Condition{ + Type: condType, + Status: status, + LastTransitionTime: metav1.Now(), + Reason: reason, + Message: message, + } +} + // NewContainerRuntimeConfigCondition returns an instance of a ContainerRuntimeConfigCondition func NewContainerRuntimeConfigCondition(condType mcfgv1.ContainerRuntimeConfigStatusConditionType, status corev1.ConditionStatus, message string) *mcfgv1.ContainerRuntimeConfigCondition { return &mcfgv1.ContainerRuntimeConfigCondition{ diff --git a/pkg/controller/bootstrap/bootstrap.go b/pkg/controller/bootstrap/bootstrap.go index 3129980b6a..3cebb1a357 100644 --- a/pkg/controller/bootstrap/bootstrap.go +++ b/pkg/controller/bootstrap/bootstrap.go @@ -19,6 +19,7 @@ import ( "k8s.io/klog/v2" apicfgv1 "github.com/openshift/api/config/v1" + apicfgv1alpha1 "github.com/openshift/api/config/v1alpha1" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" @@ -72,20 +73,24 @@ func (b *Bootstrap) Run(destDir string) error { mcfgv1.Install(scheme) apioperatorsv1alpha1.Install(scheme) apicfgv1.Install(scheme) + apicfgv1alpha1.Install(scheme) codecFactory := serializer.NewCodecFactory(scheme) - decoder := codecFactory.UniversalDecoder(mcfgv1.GroupVersion, apioperatorsv1alpha1.GroupVersion, apicfgv1.GroupVersion) - - var cconfig *mcfgv1.ControllerConfig - var featureGate *apicfgv1.FeatureGate - var nodeConfig *apicfgv1.Node - var kconfigs []*mcfgv1.KubeletConfig - var pools []*mcfgv1.MachineConfigPool - var configs []*mcfgv1.MachineConfig - var crconfigs []*mcfgv1.ContainerRuntimeConfig - var icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy - var idmsRules []*apicfgv1.ImageDigestMirrorSet - var itmsRules []*apicfgv1.ImageTagMirrorSet - var imgCfg *apicfgv1.Image + decoder := codecFactory.UniversalDecoder(mcfgv1.GroupVersion, apioperatorsv1alpha1.GroupVersion, apicfgv1.GroupVersion, apicfgv1alpha1.GroupVersion) + + var ( + cconfig *mcfgv1.ControllerConfig + featureGate *apicfgv1.FeatureGate + nodeConfig *apicfgv1.Node + kconfigs []*mcfgv1.KubeletConfig + pools []*mcfgv1.MachineConfigPool + configs []*mcfgv1.MachineConfig + crconfigs []*mcfgv1.ContainerRuntimeConfig + icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy + idmsRules []*apicfgv1.ImageDigestMirrorSet + itmsRules []*apicfgv1.ImageTagMirrorSet + clusterImagePolicies []*apicfgv1alpha1.ClusterImagePolicy + imgCfg *apicfgv1.Image + ) for _, info := range infos { if info.IsDir() { continue @@ -132,6 +137,8 @@ func (b *Bootstrap) Run(destDir string) error { itmsRules = append(itmsRules, obj) case *apicfgv1.Image: imgCfg = obj + case *apicfgv1alpha1.ClusterImagePolicy: + clusterImagePolicies = append(clusterImagePolicies, obj) case *apicfgv1.FeatureGate: if obj.GetName() == ctrlcommon.ClusterFeatureInstanceName { featureGate = obj @@ -168,7 +175,7 @@ func (b *Bootstrap) Run(destDir string) error { configs = append(configs, iconfigs...) - rconfigs, err := containerruntimeconfig.RunImageBootstrap(b.templatesDir, cconfig, pools, icspRules, idmsRules, itmsRules, imgCfg, fgAccess) + rconfigs, err := containerruntimeconfig.RunImageBootstrap(b.templatesDir, cconfig, pools, icspRules, idmsRules, itmsRules, imgCfg, clusterImagePolicies, fgAccess) if err != nil { return err } @@ -177,7 +184,7 @@ func (b *Bootstrap) Run(destDir string) error { configs = append(configs, rconfigs...) if len(crconfigs) > 0 { - containerRuntimeConfigs, err := containerruntimeconfig.RunContainerRuntimeBootstrap(b.templatesDir, crconfigs, cconfig, pools, fgAccess) + containerRuntimeConfigs, err := containerruntimeconfig.RunContainerRuntimeBootstrap(b.templatesDir, crconfigs, cconfig, pools) if err != nil { return err } diff --git a/pkg/controller/bootstrap/testdata/bootstrap/featuregate.yaml b/pkg/controller/bootstrap/testdata/bootstrap/featuregate.yaml index 57bcab9d54..eaeda285cd 100644 --- a/pkg/controller/bootstrap/testdata/bootstrap/featuregate.yaml +++ b/pkg/controller/bootstrap/testdata/bootstrap/featuregate.yaml @@ -7,3 +7,5 @@ status: - version: 0.0.1-snapshot enabled: - name: OpenShiftPodSecurityAdmission + disabled: + - name: SigstoreImageVerification diff --git a/pkg/controller/container-runtime-config/container_runtime_config_bootstrap.go b/pkg/controller/container-runtime-config/container_runtime_config_bootstrap.go index 824fe2158e..3fa43bd263 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_bootstrap.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_bootstrap.go @@ -4,7 +4,6 @@ import ( "fmt" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" - "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" "github.com/openshift/machine-config-operator/pkg/version" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -13,7 +12,7 @@ import ( ) // RunContainerRuntimeBootstrap generates ignition configs at bootstrap -func RunContainerRuntimeBootstrap(templateDir string, crconfigs []*mcfgv1.ContainerRuntimeConfig, controllerConfig *mcfgv1.ControllerConfig, mcpPools []*mcfgv1.MachineConfigPool, featureGateAccess featuregates.FeatureGateAccess) ([]*mcfgv1.MachineConfig, error) { //nolint:revive,unparam +func RunContainerRuntimeBootstrap(templateDir string, crconfigs []*mcfgv1.ContainerRuntimeConfig, controllerConfig *mcfgv1.ControllerConfig, mcpPools []*mcfgv1.MachineConfigPool) ([]*mcfgv1.MachineConfig, error) { var res []*mcfgv1.MachineConfig managedKeyExist := make(map[string]bool) for _, cfg := range crconfigs { diff --git a/pkg/controller/container-runtime-config/container_runtime_config_bootstrap_test.go b/pkg/controller/container-runtime-config/container_runtime_config_bootstrap_test.go index 4a07aed983..5efeebc07d 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_bootstrap_test.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_bootstrap_test.go @@ -29,7 +29,7 @@ func TestAddKubeletCfgAfterBootstrapKubeletCfg(t *testing.T) { f.mccrLister = append(f.mccrLister, ctrcfg) f.objects = append(f.objects, ctrcfg) - mcs, err := RunContainerRuntimeBootstrap("../../../templates", []*mcfgv1.ContainerRuntimeConfig{ctrcfg}, cc, pools, f.fgAccess) + mcs, err := RunContainerRuntimeBootstrap("../../../templates", []*mcfgv1.ContainerRuntimeConfig{ctrcfg}, cc, pools) require.NoError(t, err) require.Len(t, mcs, 1) diff --git a/pkg/controller/container-runtime-config/container_runtime_config_controller.go b/pkg/controller/container-runtime-config/container_runtime_config_controller.go index edfbeb70b1..4e760d9cad 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller.go @@ -9,15 +9,22 @@ import ( "time" "github.com/clarketm/json" + signature "github.com/containers/image/v5/signature" ign3types "github.com/coreos/ignition/v2/config/v3_4/types" apicfgv1 "github.com/openshift/api/config/v1" + apicfgv1alpha1 "github.com/openshift/api/config/v1alpha1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" configclientset "github.com/openshift/client-go/config/clientset/versioned" + configinformers "github.com/openshift/client-go/config/informers/externalversions" cligoinformersv1 "github.com/openshift/client-go/config/informers/externalversions/config/v1" cligolistersv1 "github.com/openshift/client-go/config/listers/config/v1" + cligolistersv1alpha1 "github.com/openshift/client-go/config/listers/config/v1alpha1" + operatorinformersv1alpha1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1alpha1" + operatorlistersv1alpha1 "github.com/openshift/client-go/operator/listers/operator/v1alpha1" "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" + runtimeutils "github.com/openshift/runtime-utils/pkg/registries" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" @@ -41,6 +48,7 @@ import ( "github.com/openshift/client-go/machineconfiguration/clientset/versioned/scheme" mcfginformersv1 "github.com/openshift/client-go/machineconfiguration/informers/externalversions/machineconfiguration/v1" mcfglistersv1 "github.com/openshift/client-go/machineconfiguration/listers/machineconfiguration/v1" + apihelpers "github.com/openshift/machine-config-operator/pkg/apihelpers" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" mtmpl "github.com/openshift/machine-config-operator/pkg/controller/template" "github.com/openshift/machine-config-operator/pkg/version" @@ -98,6 +106,11 @@ type Controller struct { itmsLister cligolistersv1.ImageTagMirrorSetLister itmsListerSynced cache.InformerSynced + configInformerFactory configinformers.SharedInformerFactory + clusterImagePolicyLister cligolistersv1alpha1.ClusterImagePolicyLister + clusterImagePolicyListerSynced cache.InformerSynced + addedPolicyObservers bool + mcpLister mcfglistersv1.MachineConfigPoolLister mcpListerSynced cache.InformerSynced @@ -119,6 +132,7 @@ func New( imgInformer cligoinformersv1.ImageInformer, idmsInformer cligoinformersv1.ImageDigestMirrorSetInformer, itmsInformer cligoinformersv1.ImageTagMirrorSetInformer, + configInformerFactory configinformers.SharedInformerFactory, icspInformer operatorinformersv1alpha1.ImageContentSourcePolicyInformer, clusterVersionInformer cligoinformersv1.ClusterVersionInformer, kubeClient clientset.Interface, @@ -199,6 +213,8 @@ func New( ctrl.featureGateAccess = featureGateAccess + ctrl.configInformerFactory = configInformerFactory + return ctrl } @@ -207,9 +223,18 @@ func (ctrl *Controller) Run(workers int, stopCh <-chan struct{}) { defer utilruntime.HandleCrash() defer ctrl.queue.ShutDown() defer ctrl.imgQueue.ShutDown() + listerCaches := []cache.InformerSynced{ctrl.mcpListerSynced, ctrl.mccrListerSynced, ctrl.ccListerSynced, + ctrl.imgListerSynced, ctrl.icspListerSynced, ctrl.idmsListerSynced, ctrl.itmsListerSynced, ctrl.clusterVersionListerSynced} + + if ctrl.sigstoreAPIEnabled() { + ctrl.addImagePolicyObservers() + klog.Info("addded image policy observers with sigstore featuregate enabled") + ctrl.configInformerFactory.Start(stopCh) + listerCaches = append(listerCaches, ctrl.clusterImagePolicyListerSynced) + ctrl.addedPolicyObservers = true + } - if !cache.WaitForCacheSync(stopCh, ctrl.mcpListerSynced, ctrl.mccrListerSynced, ctrl.ccListerSynced, - ctrl.imgListerSynced, ctrl.icspListerSynced, ctrl.idmsListerSynced, ctrl.itmsListerSynced, ctrl.clusterVersionListerSynced) { + if !cache.WaitForCacheSync(stopCh, listerCaches...) { return } @@ -284,6 +309,37 @@ func (ctrl *Controller) itmsConfDeleted(_ interface{}) { ctrl.imgQueue.Add("openshift-config") } +func (ctrl *Controller) addImagePolicyObservers() { + ctrl.configInformerFactory.Config().V1alpha1().ClusterImagePolicies().Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: ctrl.clusterImagePolicyAdded, + UpdateFunc: ctrl.clusterImagePolicyUpdated, + DeleteFunc: ctrl.clusterImagePolicyDeleted, + }) + ctrl.clusterImagePolicyLister = ctrl.configInformerFactory.Config().V1alpha1().ClusterImagePolicies().Lister() + ctrl.clusterImagePolicyListerSynced = ctrl.configInformerFactory.Config().V1alpha1().ClusterImagePolicies().Informer().HasSynced +} + +func (ctrl *Controller) clusterImagePolicyAdded(_ interface{}) { + ctrl.imgQueue.Add("openshift-config") +} + +func (ctrl *Controller) clusterImagePolicyUpdated(_, _ interface{}) { + ctrl.imgQueue.Add("openshift-config") +} + +func (ctrl *Controller) clusterImagePolicyDeleted(_ interface{}) { + ctrl.imgQueue.Add("openshift-config") +} + +func (ctrl *Controller) sigstoreAPIEnabled() bool { + featureGates, err := ctrl.featureGateAccess.CurrentFeatureGates() + if err != nil { + klog.Infof("error getting current featuregates: %v", err) + return false + } + return featureGates.Enabled(apicfgv1.FeatureGateSigstoreImageVerification) +} + func (ctrl *Controller) updateContainerRuntimeConfig(oldObj, newObj interface{}) { oldCtrCfg := oldObj.(*mcfgv1.ContainerRuntimeConfig) newCtrCfg := newObj.(*mcfgv1.ContainerRuntimeConfig) @@ -790,7 +846,18 @@ func (ctrl *Controller) syncImageConfig(key string) error { var ( registriesBlocked, policyBlocked, allowedRegs []string releaseImage string + clusterImagePolicies []*apicfgv1alpha1.ClusterImagePolicy + clusterScopePolicies map[string]signature.PolicyRequirements ) + + if ctrl.sigstoreAPIEnabled() && ctrl.addedPolicyObservers { + // Find all ClusterImagePolicy objects + clusterImagePolicies, err = ctrl.clusterImagePolicyLister.List(labels.Everything()) + if err != nil && errors.IsNotFound(err) { + clusterImagePolicies = []*apicfgv1alpha1.ClusterImagePolicy{} + } + } + if clusterVersionCfg != nil { // The possibility of releaseImage being "" is very unlikely, will only happen if clusterVersionCfg is nil. If this happens // then there is something very wrong with the cluster and in that situation it would be best to fail here till clusterVersionCfg @@ -803,6 +870,12 @@ func (ctrl *Controller) syncImageConfig(key string) error { } else if err == errParsingReference { return err } + + // Get the valid clusterimagepolicies, update to the status if has skipped scopes + // skip the scope if the it is for release image repo + if clusterScopePolicies, err = getValidScopePolicies(clusterImagePolicies, releaseImage, ctrl); err != nil { + return err + } } // Get ControllerConfig @@ -832,54 +905,15 @@ func (ctrl *Controller) syncImageConfig(key string) error { if err := retry.RetryOnConflict(updateBackoff, func() error { registriesIgn, err := registriesConfigIgnition(ctrl.templatesDir, controllerConfig, role, releaseImage, imgcfg.Spec.RegistrySources.InsecureRegistries, registriesBlocked, policyBlocked, allowedRegs, - imgcfg.Spec.RegistrySources.ContainerRuntimeSearchRegistries, icspRules, idmsRules, itmsRules, ctrl.featureGateAccess) + imgcfg.Spec.RegistrySources.ContainerRuntimeSearchRegistries, icspRules, idmsRules, itmsRules, clusterScopePolicies) if err != nil { return err } - rawRegistriesIgn, err := json.Marshal(registriesIgn) + + applied, err = ctrl.syncIgnitionConfig(managedKey, registriesIgn, pool, ownerReferenceImageConfig(imgcfg)) if err != nil { - return fmt.Errorf("could not encode registries Ignition config: %w", err) - } - mc, err := ctrl.client.MachineconfigurationV1().MachineConfigs().Get(context.TODO(), managedKey, metav1.GetOptions{}) - if err != nil && !errors.IsNotFound(err) { - return fmt.Errorf("could not find MachineConfig: %w", err) - } - isNotFound := errors.IsNotFound(err) - if !isNotFound && equality.Semantic.DeepEqual(rawRegistriesIgn, mc.Spec.Config.Raw) { - // if the configuration for the registries is equal, we still need to compare - // the generated controller version because during an upgrade we need a new one - mcCtrlVersion := mc.Annotations[ctrlcommon.GeneratedByControllerVersionAnnotationKey] - if mcCtrlVersion == version.Hash { - applied = false - return nil - } - } - if isNotFound { - tempIgnCfg := ctrlcommon.NewIgnConfig() - mc, err = ctrlcommon.MachineConfigFromIgnConfig(role, managedKey, tempIgnCfg) - if err != nil { - return fmt.Errorf("could not create MachineConfig from new Ignition config: %w", err) - } - } - mc.Spec.Config.Raw = rawRegistriesIgn - mc.ObjectMeta.Annotations = map[string]string{ - ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash, - } - mc.ObjectMeta.OwnerReferences = []metav1.OwnerReference{ - { - APIVersion: apicfgv1.SchemeGroupVersion.String(), - Kind: "Image", - Name: imgcfg.Name, - UID: imgcfg.UID, - }, - } - // Create or Update, on conflict retry - if isNotFound { - _, err = ctrl.client.MachineconfigurationV1().MachineConfigs().Create(context.TODO(), mc, metav1.CreateOptions{}) - } else { - _, err = ctrl.client.MachineconfigurationV1().MachineConfigs().Update(context.TODO(), mc, metav1.UpdateOptions{}) + return fmt.Errorf("could not sync registries Ignition config: %w", err) } - return err }); err != nil { return fmt.Errorf("could not Create/Update MachineConfig: %w", err) @@ -892,13 +926,56 @@ func (ctrl *Controller) syncImageConfig(key string) error { return nil } +func (ctrl *Controller) syncIgnitionConfig(managedKey string, ignFile *ign3types.Config, pool *mcfgv1.MachineConfigPool, ownerRef metav1.OwnerReference) (bool, error) { + rawIgn, err := json.Marshal(ignFile) + if err != nil { + return false, fmt.Errorf("could not encode Ignition config: %w", err) + } + mc, err := ctrl.client.MachineconfigurationV1().MachineConfigs().Get(context.TODO(), managedKey, metav1.GetOptions{}) + if err != nil && !errors.IsNotFound(err) { + return false, fmt.Errorf("could not find MachineConfig: %w", err) + } + isNotFound := errors.IsNotFound(err) + if !isNotFound && equality.Semantic.DeepEqual(rawIgn, mc.Spec.Config.Raw) { + // if the configuration for the registries is equal, we still need to compare + // the generated controller version because during an upgrade we need a new one + mcCtrlVersion := mc.Annotations[ctrlcommon.GeneratedByControllerVersionAnnotationKey] + if mcCtrlVersion == version.Hash { + return false, nil + } + } + if isNotFound { + tempIgnCfg := ctrlcommon.NewIgnConfig() + mc, err = ctrlcommon.MachineConfigFromIgnConfig(pool.Name, managedKey, tempIgnCfg) + if err != nil { + return false, fmt.Errorf("could not create MachineConfig from new Ignition config: %w", err) + } + } + mc.Spec.Config.Raw = rawIgn + mc.ObjectMeta.Annotations = map[string]string{ + ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash, + } + mc.ObjectMeta.OwnerReferences = []metav1.OwnerReference{ownerRef} + // Create or Update, on conflict retry + if isNotFound { + _, err = ctrl.client.MachineconfigurationV1().MachineConfigs().Create(context.TODO(), mc, metav1.CreateOptions{}) + } else { + _, err = ctrl.client.MachineconfigurationV1().MachineConfigs().Update(context.TODO(), mc, metav1.UpdateOptions{}) + } + + return true, err +} + func registriesConfigIgnition(templateDir string, controllerConfig *mcfgv1.ControllerConfig, role, releaseImage string, insecureRegs, registriesBlocked, policyBlocked, allowedRegs, searchRegs []string, - icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy, idmsRules []*apicfgv1.ImageDigestMirrorSet, itmsRules []*apicfgv1.ImageTagMirrorSet, featureGateAccess featuregates.FeatureGateAccess) (*ign3types.Config, error) { //nolint:revive,unparam + icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy, idmsRules []*apicfgv1.ImageDigestMirrorSet, itmsRules []*apicfgv1.ImageTagMirrorSet, + clusterScopePolicies map[string]signature.PolicyRequirements) (*ign3types.Config, error) { var ( - registriesTOML []byte - policyJSON []byte + registriesTOML []byte + policyJSON []byte + sigstoreRegistriesConfigYaml []byte + err error ) // Generate the original registries config @@ -920,7 +997,7 @@ func registriesConfigIgnition(templateDir string, controllerConfig *mcfgv1.Contr return nil, fmt.Errorf("could not update registries config with new changes: %w", err) } } - if policyBlocked != nil || allowedRegs != nil { + if policyBlocked != nil || allowedRegs != nil || len(clusterScopePolicies) > 0 { if originalPolicyIgn.Contents.Source == nil { return nil, fmt.Errorf("original policy json is empty") } @@ -928,14 +1005,21 @@ func registriesConfigIgnition(templateDir string, controllerConfig *mcfgv1.Contr if err != nil { return nil, fmt.Errorf("could not decode original policy json: %w", err) } - policyJSON, err = updatePolicyJSON(contents, policyBlocked, allowedRegs, releaseImage) + policyJSON, err = updatePolicyJSON(contents, policyBlocked, allowedRegs, releaseImage, clusterScopePolicies) if err != nil { return nil, fmt.Errorf("could not update policy json with new changes: %w", err) } + // generates configuration under /etc/containers/registries.d to enable sigstore verification + sigstoreRegistriesConfigYaml, err = generateSigstoreRegistriesdConfig(clusterScopePolicies) + if err != nil { + return nil, err + } } + generatedConfigFileList := []generatedConfigFile{ {filePath: registriesConfigPath, data: registriesTOML}, {filePath: policyConfigPath, data: policyJSON}, + {filePath: sigstoreRegistriesConfigFilePath, data: sigstoreRegistriesConfigYaml}, } if searchRegs != nil { generatedConfigFileList = append(generatedConfigFileList, updateSearchRegistriesConfig(searchRegs)...) @@ -945,16 +1029,83 @@ func registriesConfigIgnition(templateDir string, controllerConfig *mcfgv1.Contr return ®istriesIgn, nil } +// getValidScopePolicies returns a map[scope]policyRequirement from ClusterImagePolicy +// not add scope to the map if it is (or super scope of) release image repo, and syncs to status +func getValidScopePolicies(clusterImagePolicies []*apicfgv1alpha1.ClusterImagePolicy, releaseImage string, ctrl *Controller) (map[string]signature.PolicyRequirements, error) { + clusterScopePolicies := make(map[string]signature.PolicyRequirements) + + // Get the repository being used by the payload from the releaseImage + ref, err := getPayloadRepo(releaseImage) + if err != nil { + return nil, errParsingReference + } + payloadRepo := ref.Name() + + for _, clusterImagePolicy := range clusterImagePolicies { + sigstoreSignedPolicyItem, err := policyItemFromSpec(clusterImagePolicy.Spec.Policy) + if err != nil { + return nil, err + } + var payloadScopes []string + for _, scope := range clusterImagePolicy.Spec.Scopes { + scopeStr := string(scope) + if runtimeutils.ScopeIsNestedInsideScope(payloadRepo, scopeStr) { + payloadScopes = append(payloadScopes, scopeStr) + continue + } + clusterScopePolicies[scopeStr] = append(clusterScopePolicies[scopeStr], sigstoreSignedPolicyItem) + } + if ctrl != nil && len(payloadScopes) > 0 { + msg := fmt.Sprintf("has conflict scope(s) %q of Openshift payload repository %s, skip the scope(s)", payloadScopes, payloadRepo) + klog.V(2).Info(msg) + ctrl.syncClusterImagePolicyStatusOnly(clusterImagePolicy.ObjectMeta.Name, apicfgv1alpha1.ImagePolicyPending, reasonConflictScopes, msg, metav1.ConditionTrue) + } + } + + return clusterScopePolicies, nil +} + +func (ctrl *Controller) syncClusterImagePolicyStatusOnly(imagepolicy, conditionType, reason, msg string, status metav1.ConditionStatus) { + statusUpdateErr := retry.RetryOnConflict(updateBackoff, func() error { + newClusterImagePolicy, getErr := ctrl.configClient.ConfigV1alpha1().ClusterImagePolicies().Get(context.TODO(), imagepolicy, metav1.GetOptions{}) + if getErr != nil { + return getErr + } + newCondition := apihelpers.NewCondition(conditionType, status, reason, msg) + if newClusterImagePolicy.GetGeneration() != newCondition.ObservedGeneration { + newCondition.ObservedGeneration = newClusterImagePolicy.GetGeneration() + } + newClusterImagePolicy.Status.Conditions = []metav1.Condition{*newCondition} + _, updateErr := ctrl.configClient.ConfigV1alpha1().ClusterImagePolicies().UpdateStatus(context.TODO(), newClusterImagePolicy, metav1.UpdateOptions{}) + return updateErr + }) + if statusUpdateErr != nil { + klog.Warningf("error updating clusterimagepolicy status: %v", statusUpdateErr) + } +} + // RunImageBootstrap generates MachineConfig objects for mcpPools that would have been generated by syncImageConfig, // except that mcfgv1.Image is not available. func RunImageBootstrap(templateDir string, controllerConfig *mcfgv1.ControllerConfig, mcpPools []*mcfgv1.MachineConfigPool, icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy, - idmsRules []*apicfgv1.ImageDigestMirrorSet, itmsRules []*apicfgv1.ImageTagMirrorSet, imgCfg *apicfgv1.Image, featureGateAccess featuregates.FeatureGateAccess) ([]*mcfgv1.MachineConfig, error) { + idmsRules []*apicfgv1.ImageDigestMirrorSet, itmsRules []*apicfgv1.ImageTagMirrorSet, imgCfg *apicfgv1.Image, clusterImagePolicies []*apicfgv1alpha1.ClusterImagePolicy, featureGateAccess featuregates.FeatureGateAccess) ([]*mcfgv1.MachineConfig, error) { var ( insecureRegs, registriesBlocked, policyBlocked, allowedRegs, searchRegs []string err error ) + clusterScopePolicies := map[string]signature.PolicyRequirements{} + featureGates, err := featureGateAccess.CurrentFeatureGates() + if err != nil { + return nil, err + } + sigstoreAPIEnabled := featureGates.Enabled(apicfgv1.FeatureGateSigstoreImageVerification) + if sigstoreAPIEnabled { + if clusterScopePolicies, err = getValidScopePolicies(clusterImagePolicies, controllerConfig.Spec.ReleaseImage, nil); err != nil { + return nil, err + } + } + // Read the search, insecure, blocked, and allowed registries from the cluster-wide Image CR if it is not nil if imgCfg != nil { insecureRegs = imgCfg.Spec.RegistrySources.InsecureRegistries @@ -976,7 +1127,7 @@ func RunImageBootstrap(templateDir string, controllerConfig *mcfgv1.ControllerCo return nil, err } registriesIgn, err := registriesConfigIgnition(templateDir, controllerConfig, role, controllerConfig.Spec.ReleaseImage, - insecureRegs, registriesBlocked, policyBlocked, allowedRegs, searchRegs, icspRules, idmsRules, itmsRules, featureGateAccess) + insecureRegs, registriesBlocked, policyBlocked, allowedRegs, searchRegs, icspRules, idmsRules, itmsRules, clusterScopePolicies) if err != nil { return nil, err } diff --git a/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go b/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go index 8719ec4130..303996487f 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go @@ -28,6 +28,7 @@ import ( ign3types "github.com/coreos/ignition/v2/config/v3_4/types" apicfgv1 "github.com/openshift/api/config/v1" + apicfgv1alpha1 "github.com/openshift/api/config/v1alpha1" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" fakeconfigv1client "github.com/openshift/client-go/config/clientset/versioned/fake" @@ -62,14 +63,15 @@ type fixture struct { imgClient *fakeconfigv1client.Clientset operatorClient *fakeoperatorclient.Clientset - ccLister []*mcfgv1.ControllerConfig - mcpLister []*mcfgv1.MachineConfigPool - mccrLister []*mcfgv1.ContainerRuntimeConfig - imgLister []*apicfgv1.Image - cvLister []*apicfgv1.ClusterVersion - icspLister []*apioperatorsv1alpha1.ImageContentSourcePolicy - idmsLister []*apicfgv1.ImageDigestMirrorSet - itmsLister []*apicfgv1.ImageTagMirrorSet + ccLister []*mcfgv1.ControllerConfig + mcpLister []*mcfgv1.MachineConfigPool + mccrLister []*mcfgv1.ContainerRuntimeConfig + imgLister []*apicfgv1.Image + cvLister []*apicfgv1.ClusterVersion + icspLister []*apioperatorsv1alpha1.ImageContentSourcePolicy + idmsLister []*apicfgv1.ImageDigestMirrorSet + itmsLister []*apicfgv1.ImageTagMirrorSet + clusterImagePolicyLister []*apicfgv1alpha1.ClusterImagePolicy actions []core.Action skipActionsValidation bool @@ -86,7 +88,7 @@ func newFixture(t *testing.T) *fixture { f.t = t f.objects = []runtime.Object{} f.fgAccess = featuregates.NewHardcodedFeatureGateAccess( - []apicfgv1.FeatureGateName{}, + []apicfgv1.FeatureGateName{apicfgv1.FeatureGateSigstoreImageVerification}, []apicfgv1.FeatureGateName{ apicfgv1.FeatureGateExternalCloudProvider, apicfgv1.FeatureGateExternalCloudProviderAzure, @@ -210,6 +212,28 @@ func newClusterVersionConfig(name, desiredImage string) *apicfgv1.ClusterVersion } } +func newClusterImagePolicyWithPublicKey(name string, scopes []string, keyData []byte) *apicfgv1alpha1.ClusterImagePolicy { + imgScopes := []apicfgv1alpha1.ImageScope{} + for _, scope := range scopes { + imgScopes = append(imgScopes, apicfgv1alpha1.ImageScope(scope)) + } + return &apicfgv1alpha1.ClusterImagePolicy{ + TypeMeta: metav1.TypeMeta{APIVersion: apicfgv1alpha1.SchemeGroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Name: name, UID: types.UID(utilrand.String(5)), Generation: 1}, + Spec: apicfgv1alpha1.ClusterImagePolicySpec{ + Scopes: imgScopes, + Policy: apicfgv1alpha1.Policy{ + RootOfTrust: apicfgv1alpha1.PolicyRootOfTrust{ + PolicyType: apicfgv1alpha1.PublicKeyRootOfTrust, + PublicKey: &apicfgv1alpha1.PublicKey{ + KeyData: keyData, + }, + }, + }, + }, + } +} + func (f *fixture) newController() *Controller { f.client = fake.NewSimpleClientset(f.objects...) f.imgClient = fakeconfigv1client.NewSimpleClientset(f.imgObjects...) @@ -225,6 +249,7 @@ func (f *fixture) newController() *Controller { ci.Config().V1().Images(), ci.Config().V1().ImageDigestMirrorSets(), ci.Config().V1().ImageTagMirrorSets(), + ci, oi.Operator().V1alpha1().ImageContentSourcePolicies(), ci.Config().V1().ClusterVersions(), k8sfake.NewSimpleClientset(), f.client, f.imgClient, @@ -238,6 +263,7 @@ func (f *fixture) newController() *Controller { c.icspListerSynced = alwaysReady c.idmsListerSynced = alwaysReady c.itmsListerSynced = alwaysReady + c.clusterImagePolicyListerSynced = alwaysReady c.clusterVersionListerSynced = alwaysReady c.eventRecorder = &record.FakeRecorder{} @@ -274,6 +300,9 @@ func (f *fixture) newController() *Controller { for _, c := range f.itmsLister { ci.Config().V1().ImageTagMirrorSets().Informer().GetIndexer().Add(c) } + for _, c := range f.clusterImagePolicyLister { + ci.Config().V1alpha1().ClusterImagePolicies().Informer().GetIndexer().Add(c) + } return c } @@ -288,7 +317,10 @@ func (f *fixture) runExpectError(mcpname string) { func (f *fixture) runController(mcpname string, expectError bool) { c := f.newController() - + if !c.addedPolicyObservers { + c.addImagePolicyObservers() + c.addedPolicyObservers = true + } err := c.syncImgHandler(mcpname) if !expectError && err != nil { f.t.Errorf("error syncing image config: %v", err) @@ -388,6 +420,10 @@ func (f *fixture) expectCreateMachineConfigAction(config *mcfgv1.MachineConfig) f.actions = append(f.actions, core.NewRootCreateAction(schema.GroupVersionResource{Resource: "machineconfigs"}, config)) } +func (f *fixture) expectDeleteMachineConfigAction(config *mcfgv1.MachineConfig) { + f.actions = append(f.actions, core.NewRootDeleteAction(schema.GroupVersionResource{Resource: "machineconfigs"}, config.Name)) +} + func (f *fixture) expectUpdateMachineConfigAction(config *mcfgv1.MachineConfig) { f.actions = append(f.actions, core.NewRootUpdateAction(schema.GroupVersionResource{Resource: "machineconfigs"}, config)) } @@ -404,7 +440,7 @@ func (f *fixture) expectUpdateContainerRuntimeConfigRoot(config *mcfgv1.Containe f.actions = append(f.actions, core.NewRootUpdateAction(schema.GroupVersionResource{Version: "v1", Group: "machineconfiguration.openshift.io", Resource: "containerruntimeconfigs"}, config)) } -func (f *fixture) verifyRegistriesConfigAndPolicyJSONContents(t *testing.T, mcName string, imgcfg *apicfgv1.Image, icsp *apioperatorsv1alpha1.ImageContentSourcePolicy, idms *apicfgv1.ImageDigestMirrorSet, itms *apicfgv1.ImageTagMirrorSet, releaseImageReg string, verifyPolicyJSON, verifySearchRegsDropin bool) { +func (f *fixture) verifyRegistriesConfigAndPolicyJSONContents(t *testing.T, mcName string, imgcfg *apicfgv1.Image, icsp *apioperatorsv1alpha1.ImageContentSourcePolicy, idms *apicfgv1.ImageDigestMirrorSet, itms *apicfgv1.ImageTagMirrorSet, imagepolicy *apicfgv1alpha1.ClusterImagePolicy, releaseImageReg string, verifyPolicyJSON, verifySearchRegsDropin, verifyImagePoliciesRegistriesConfig bool) { icsps := []*apioperatorsv1alpha1.ImageContentSourcePolicy{} if icsp != nil { icsps = append(icsps, icsp) @@ -417,12 +453,16 @@ func (f *fixture) verifyRegistriesConfigAndPolicyJSONContents(t *testing.T, mcNa if itms != nil { itmss = append(itmss, itms) } + clusterImagePolicies := []*apicfgv1alpha1.ClusterImagePolicy{} + if imagepolicy != nil { + clusterImagePolicies = append(clusterImagePolicies, imagepolicy) + } updatedMC, err := f.client.MachineconfigurationV1().MachineConfigs().Get(context.TODO(), mcName, metav1.GetOptions{}) require.NoError(t, err) - verifyRegistriesConfigAndPolicyJSONContents(t, updatedMC, mcName, imgcfg, icsps, idmss, itmss, releaseImageReg, verifyPolicyJSON, verifySearchRegsDropin) + verifyRegistriesConfigAndPolicyJSONContents(t, updatedMC, mcName, imgcfg, icsps, idmss, itmss, clusterImagePolicies, releaseImageReg, verifyPolicyJSON, verifySearchRegsDropin, verifyImagePoliciesRegistriesConfig) } -func verifyRegistriesConfigAndPolicyJSONContents(t *testing.T, mc *mcfgv1.MachineConfig, mcName string, imgcfg *apicfgv1.Image, icsps []*apioperatorsv1alpha1.ImageContentSourcePolicy, idmss []*apicfgv1.ImageDigestMirrorSet, itmss []*apicfgv1.ImageTagMirrorSet, releaseImageReg string, verifyPolicyJSON, verifySearchRegsDropin bool) { +func verifyRegistriesConfigAndPolicyJSONContents(t *testing.T, mc *mcfgv1.MachineConfig, mcName string, imgcfg *apicfgv1.Image, icsps []*apioperatorsv1alpha1.ImageContentSourcePolicy, idmss []*apicfgv1.ImageDigestMirrorSet, itmss []*apicfgv1.ImageTagMirrorSet, clusterImagePolicies []*apicfgv1alpha1.ClusterImagePolicy, releaseImageReg string, verifyPolicyJSON, verifySearchRegsDropin, verifyImagePoliciesRegistriesConfig bool) { // This is not testing updateRegistriesConfig, which has its own tests; this verifies the created object contains the expected // configuration file. // First get the valid blocked registries to ensure we don't block the registry where the release image is from @@ -435,7 +475,9 @@ func verifyRegistriesConfigAndPolicyJSONContents(t *testing.T, mc *mcfgv1.Machin ignCfg, err := ctrlcommon.ParseAndConvertConfig(mc.Spec.Config.Raw) require.NoError(t, err) - if verifyPolicyJSON && verifySearchRegsDropin { + if verifyImagePoliciesRegistriesConfig && verifyPolicyJSON && verifySearchRegsDropin { + require.Len(t, ignCfg.Storage.Files, 4) + } else if verifyPolicyJSON && verifySearchRegsDropin { // If there is a change to the policy.json AND drop-in search registries file then there will be 3 files require.Len(t, ignCfg.Storage.Files, 3) } else if verifyPolicyJSON || verifySearchRegsDropin { @@ -453,12 +495,15 @@ func verifyRegistriesConfigAndPolicyJSONContents(t *testing.T, mc *mcfgv1.Machin require.NoError(t, err) assert.Equal(t, string(expectedRegistriesConf), string(registriesConf)) + clusterScopePolicies, err := getValidScopePolicies(clusterImagePolicies, releaseImageReg, nil) + require.NoError(t, err) + // Validate the policy.json contents if a change is expected from the tests if verifyPolicyJSON { allowed = append(allowed, imgcfg.Spec.RegistrySources.AllowedRegistries...) expectedPolicyJSON, err := updatePolicyJSON(templatePolicyJSON, policyBlocked, - allowed, releaseImageReg) + allowed, releaseImageReg, clusterScopePolicies) require.NoError(t, err) policyfile := ignCfg.Storage.Files[1] if policyfile.Node.Path != policyConfigPath { @@ -473,14 +518,37 @@ func verifyRegistriesConfigAndPolicyJSONContents(t *testing.T, mc *mcfgv1.Machin // Validate the drop-in search registries file contents if a change is expected from the tests if verifySearchRegsDropin { expectedSearchRegsConf := updateSearchRegistriesConfig(imgcfg.Spec.RegistrySources.ContainerRuntimeSearchRegistries) - dropinfile := ignCfg.Storage.Files[2] - if dropinfile.Node.Path != searchRegDropInFilePath { - dropinfile = ignCfg.Storage.Files[1] + foundFile := false + for _, dropinfile := range ignCfg.Storage.Files { + if dropinfile.Node.Path == searchRegDropInFilePath { + foundFile = true + searchRegsConf, err := ctrlcommon.DecodeIgnitionFileContents(dropinfile.Contents.Source, dropinfile.Contents.Compression) + require.NoError(t, err) + assert.Equal(t, string(expectedSearchRegsConf[0].data), string(searchRegsConf)) + } + } + if !foundFile { + t.Errorf("Expected to find drop-in search registries file in machineconfig %s", mcName) } - assert.Equal(t, searchRegDropInFilePath, dropinfile.Node.Path) - searchRegsConf, err := ctrlcommon.DecodeIgnitionFileContents(dropinfile.Contents.Source, dropinfile.Contents.Compression) + } + + if verifyImagePoliciesRegistriesConfig { + expectedRegistriesConfd, err := generateSigstoreRegistriesdConfig(clusterScopePolicies) require.NoError(t, err) - assert.Equal(t, string(expectedSearchRegsConf[0].data), string(searchRegsConf)) + foundFile := false + + for _, f := range ignCfg.Storage.Files { + if f.Node.Path == sigstoreRegistriesConfigFilePath { + foundFile = true + require.Equal(t, sigstoreRegistriesConfigFilePath, f.Node.Path) + registriesYaml, err := ctrlcommon.DecodeIgnitionFileContents(f.Contents.Source, f.Contents.Compression) + require.NoError(t, err) + assert.Equal(t, string(expectedRegistriesConfd), string(registriesYaml)) + } + } + if !foundFile { + t.Errorf("Expected to find sigstore registries config file in machineconfig %s", mcName) + } } } @@ -647,7 +715,7 @@ func TestImageConfigCreate(t *testing.T) { f.run("cluster") for _, mcName := range []string{mcs1.Name, mcs2.Name} { - f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, nil, nil, nil, cc.Spec.ReleaseImage, true, true) + f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, nil, nil, nil, nil, cc.Spec.ReleaseImage, true, true, false) } }) } @@ -702,7 +770,7 @@ func TestImageConfigUpdate(t *testing.T) { close(stopCh) for _, mcName := range []string{mcs1Update.Name, mcs2Update.Name} { - f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, nil, nil, nil, cc.Spec.ReleaseImage, true, true) + f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, nil, nil, nil, nil, cc.Spec.ReleaseImage, true, true, false) } // Perform Update @@ -743,7 +811,7 @@ func TestImageConfigUpdate(t *testing.T) { close(stopCh) for _, mcName := range []string{mcs1Update.Name, mcs2Update.Name} { - f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfgUpdate, nil, nil, nil, cc.Spec.ReleaseImage, true, true) + f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfgUpdate, nil, nil, nil, nil, cc.Spec.ReleaseImage, true, true, false) } }) } @@ -803,7 +871,7 @@ func TestICSPUpdate(t *testing.T) { close(stopCh) for _, mcName := range []string{mcs1Update.Name, mcs2Update.Name} { - f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, icsp, nil, nil, cc.Spec.ReleaseImage, false, false) + f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, icsp, nil, nil, nil, cc.Spec.ReleaseImage, false, false, false) } // Perform Update @@ -848,7 +916,7 @@ func TestICSPUpdate(t *testing.T) { close(stopCh) for _, mcName := range []string{mcs1Update.Name, mcs2Update.Name} { - f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, icspUpdate, nil, nil, cc.Spec.ReleaseImage, false, false) + f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, icspUpdate, nil, nil, nil, cc.Spec.ReleaseImage, false, false, false) } }) } @@ -906,7 +974,7 @@ func TestIDMSUpdate(t *testing.T) { close(stopCh) for _, mcName := range []string{mcs1Update.Name, mcs2Update.Name} { - f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, nil, idms, nil, cc.Spec.ReleaseImage, false, false) + f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, nil, idms, nil, nil, cc.Spec.ReleaseImage, false, false, false) } // Perform Update @@ -951,7 +1019,7 @@ func TestIDMSUpdate(t *testing.T) { close(stopCh) for _, mcName := range []string{mcs1Update.Name, mcs2Update.Name} { - f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, nil, idmsUpdate, nil, cc.Spec.ReleaseImage, false, false) + f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, nil, idmsUpdate, nil, nil, cc.Spec.ReleaseImage, false, false, false) } }) } @@ -1009,7 +1077,7 @@ func TestITMSUpdate(t *testing.T) { close(stopCh) for _, mcName := range []string{mcs1Update.Name, mcs2Update.Name} { - f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, nil, nil, itms, cc.Spec.ReleaseImage, false, false) + f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, nil, nil, itms, nil, cc.Spec.ReleaseImage, false, false, false) } // Perform Update @@ -1054,18 +1122,20 @@ func TestITMSUpdate(t *testing.T) { close(stopCh) for _, mcName := range []string{mcs1Update.Name, mcs2Update.Name} { - f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, nil, nil, itmsUpdate, cc.Spec.ReleaseImage, false, false) + f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, nil, nil, itmsUpdate, nil, cc.Spec.ReleaseImage, false, false, false) } }) } } func TestRunImageBootstrap(t *testing.T) { + testClusterImagePolicy := clusterImagePolicyTestCRs()["test-cr0"] for _, platform := range []apicfgv1.PlatformType{apicfgv1.AWSPlatformType, apicfgv1.NonePlatformType, "unrecognized"} { for _, tc := range []struct { - icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy - idmsRules []*apicfgv1.ImageDigestMirrorSet - itmsRules []*apicfgv1.ImageTagMirrorSet + icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy + idmsRules []*apicfgv1.ImageDigestMirrorSet + itmsRules []*apicfgv1.ImageTagMirrorSet + clusterImagePolicies []*apicfgv1alpha1.ClusterImagePolicy }{ { idmsRules: []*apicfgv1.ImageDigestMirrorSet{ @@ -1092,6 +1162,11 @@ func TestRunImageBootstrap(t *testing.T) { }), }, }, + { + clusterImagePolicies: []*apicfgv1alpha1.ClusterImagePolicy{ + &testClusterImagePolicy, + }, + }, } { t.Run(string(platform), func(t *testing.T) { @@ -1103,16 +1178,24 @@ func TestRunImageBootstrap(t *testing.T) { // Adding the release-image registry "release-reg.io" to the list of blocked registries to ensure that is it not added to // both registries.conf and policy.json as blocked imgCfg := newImageConfig("cluster", &apicfgv1.RegistrySources{InsecureRegistries: []string{"insecure-reg-1.io", "insecure-reg-2.io"}, BlockedRegistries: []string{"blocked-reg.io", "release-reg.io"}, ContainerRuntimeSearchRegistries: []string{"search-reg.io"}}) + // set FeatureGateSigstoreImageVerification enabled for testing + fgAccess := featuregates.NewHardcodedFeatureGateAccess([]apicfgv1.FeatureGateName{apicfgv1.FeatureGateSigstoreImageVerification}, []apicfgv1.FeatureGateName{}) - fgAccess := featuregates.NewHardcodedFeatureGateAccess([]apicfgv1.FeatureGateName{}, []apicfgv1.FeatureGateName{}) - - mcs, err := RunImageBootstrap("../../../templates", cc, pools, tc.icspRules, tc.idmsRules, tc.itmsRules, imgCfg, fgAccess) + mcs, err := RunImageBootstrap("../../../templates", cc, pools, tc.icspRules, tc.idmsRules, tc.itmsRules, imgCfg, tc.clusterImagePolicies, fgAccess) require.NoError(t, err) - require.Len(t, mcs, len(pools)) + if tc.clusterImagePolicies != nil { + require.Len(t, mcs, len(pools)) + } else { + require.Len(t, mcs, len(pools)) + } for i := range pools { keyReg, _ := getManagedKeyReg(pools[i], nil) - verifyRegistriesConfigAndPolicyJSONContents(t, mcs[i], keyReg, imgCfg, tc.icspRules, tc.idmsRules, tc.itmsRules, cc.Spec.ReleaseImage, true, true) + if tc.clusterImagePolicies == nil { + verifyRegistriesConfigAndPolicyJSONContents(t, mcs[i], keyReg, imgCfg, tc.icspRules, tc.idmsRules, tc.itmsRules, tc.clusterImagePolicies, cc.Spec.ReleaseImage, true, true, false) + } else { + verifyRegistriesConfigAndPolicyJSONContents(t, mcs[i], keyReg, imgCfg, tc.icspRules, tc.idmsRules, tc.itmsRules, tc.clusterImagePolicies, cc.Spec.ReleaseImage, true, true, true) + } } }) } @@ -1664,3 +1747,49 @@ func TestCleanUpDuplicatedMC(t *testing.T) { }) } } + +func TestClusterImagePolicyCreate(t *testing.T) { + for _, platform := range []apicfgv1.PlatformType{apicfgv1.AWSPlatformType, apicfgv1.NonePlatformType, "unrecognized"} { + t.Run(string(platform), func(t *testing.T) { + f := newFixture(t) + + cc := newControllerConfig(ctrlcommon.ControllerConfigName, platform) + mcp := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0") + mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") + imgcfg1 := newImageConfig("cluster", &apicfgv1.RegistrySources{InsecureRegistries: []string{"blah.io"}, AllowedRegistries: []string{"example.com"}, ContainerRuntimeSearchRegistries: []string{"search-reg.io"}}) + + cvcfg1 := newClusterVersionConfig("version", "test.io/myuser/myimage:test") + keyReg1, _ := getManagedKeyReg(mcp, nil) + keyReg2, _ := getManagedKeyReg(mcp2, nil) + + mcs1 := helpers.NewMachineConfig(keyReg1, map[string]string{"node-role": "master"}, "dummy://", []ign3types.File{{}}) + mcs2 := helpers.NewMachineConfig(keyReg2, map[string]string{"node-role": "worker"}, "dummy://", []ign3types.File{{}}) + + clusterimgPolicy := newClusterImagePolicyWithPublicKey("image-policy", []string{"example.com"}, []byte("foo bar")) + f.ccLister = append(f.ccLister, cc) + f.mcpLister = append(f.mcpLister, mcp) + f.mcpLister = append(f.mcpLister, mcp2) + f.imgLister = append(f.imgLister, imgcfg1) + f.clusterImagePolicyLister = append(f.clusterImagePolicyLister, clusterimgPolicy) + f.cvLister = append(f.cvLister, cvcfg1) + f.imgObjects = append(f.imgObjects, imgcfg1) + + f.expectGetMachineConfigAction(mcs1) + f.expectGetMachineConfigAction(mcs1) + f.expectGetMachineConfigAction(mcs1) + f.expectCreateMachineConfigAction(mcs1) + + f.expectGetMachineConfigAction(mcs2) + f.expectGetMachineConfigAction(mcs2) + f.expectGetMachineConfigAction(mcs2) + + f.expectCreateMachineConfigAction(mcs2) + + f.run("") + + for _, mcName := range []string{mcs1.Name, mcs2.Name} { + f.verifyRegistriesConfigAndPolicyJSONContents(t, mcName, imgcfg1, nil, nil, nil, clusterimgPolicy, cc.Spec.ReleaseImage, true, true, true) + } + }) + } +} diff --git a/pkg/controller/container-runtime-config/helpers.go b/pkg/controller/container-runtime-config/helpers.go index 1e302fcd44..56ba4607a5 100644 --- a/pkg/controller/container-runtime-config/helpers.go +++ b/pkg/controller/container-runtime-config/helpers.go @@ -19,7 +19,9 @@ import ( "github.com/containers/image/v5/types" storageconfig "github.com/containers/storage/pkg/config" ign3types "github.com/coreos/ignition/v2/config/v3_4/types" + "github.com/ghodss/yaml" apicfgv1 "github.com/openshift/api/config/v1" + apicfgv1alpha1 "github.com/openshift/api/config/v1alpha1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" "github.com/openshift/runtime-utils/pkg/registries" runtimeutils "github.com/openshift/runtime-utils/pkg/registries" @@ -48,14 +50,17 @@ const ( crioDropInFilePathPidsLimit = "/etc/crio/crio.conf.d/01-ctrcfg-pidsLimit" crioDropInFilePathLogSizeMax = "/etc/crio/crio.conf.d/01-ctrcfg-logSizeMax" CRIODropInFilePathDefaultRuntime = "/etc/crio/crio.conf.d/01-ctrcfg-defaultRuntime" + imagepolicyType = "sigstoreSigned" + sigstoreRegistriesConfigFilePath = "/etc/containers/registries.d/sigstore-registries.yaml" ) var ( - errParsingReference = errors.New("error parsing reference of release image") // sourceRegex and mirrorRegex pattern should stay the same with https://github.com/openshift/api/blob/ef62af078a9387e739abd99ec1d80e9129bb5475/config/v1/types_image_digest_mirror_set.go // Validation the source and mirror format for IDMS/ITMS already exists in the CRD. We need to keep this regex validation for ICSP - sourceRegex = regexp.MustCompile(`^\*(?:\.(?:[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]+)+)?)+)?$`) - mirrorRegex = regexp.MustCompile(`^((?:[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]+)+)?)+)?$`) + sourceRegex = regexp.MustCompile(`^\*(?:\.(?:[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]+)+)?)+)?$`) + mirrorRegex = regexp.MustCompile(`^((?:[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]+)+)?)+)?$`) + errParsingReference = errors.New("error parsing reference of release image") + reasonConflictScopes = "ConflictScopes" ) // TOML-friendly explicit tables used for conversions. @@ -116,6 +121,13 @@ type tomlConfigCRIODefaultRuntime struct { } `toml:"crio"` } +type dockerConfig struct { + UseSigstoreAttachments bool `json:"use-sigstore-attachments,omitempty"` +} +type registriesConfig struct { + Docker map[string]dockerConfig `json:"docker,omitempty"` +} + // generatedConfigFile is a struct that holds the filepath and data of the various configs // Using a struct array ensures that the order of the ignition files always stay the same // ensuring that double MCs are not created due to a change in the order @@ -480,7 +492,7 @@ func updateRegistriesConfig(data []byte, internalInsecure, internalBlocked []str // It also returns an error if both allowed and blocked registries are set // WARNING: This can not safely edit policy files with arbitrary complexity, especially files which include signedBy // requirements. It expects the input policy to be generated by templates in this project. -func updatePolicyJSON(data []byte, internalBlocked, internalAllowed []string, releaseImage string) ([]byte, error) { +func updatePolicyJSON(data []byte, internalBlocked, internalAllowed []string, releaseImage string, clusterScopePolicies map[string]signature.PolicyRequirements) ([]byte, error) { if len(internalAllowed) != 0 && len(internalBlocked) != 0 { payloadRepo, err := getPayloadRepo(releaseImage) if err != nil { @@ -492,10 +504,14 @@ func updatePolicyJSON(data []byte, internalBlocked, internalAllowed []string, re return nil, fmt.Errorf("invalid images config: only one of AllowedRegistries or BlockedRegistries may be specified") } } + + if err := validateClusterImagePolicyWithAllowedBlockedRegistries(clusterScopePolicies, internalAllowed, internalBlocked); err != nil { + return nil, err + } // Return original data if neither allowed or blocked registries are configured // Note: this is just for testing, the controller does not call this function till // either allowed or blocked registries are configured - if internalAllowed == nil && internalBlocked == nil { + if internalAllowed == nil && internalBlocked == nil && len(clusterScopePolicies) == 0 { return data, nil } @@ -515,6 +531,10 @@ func updatePolicyJSON(data []byte, internalBlocked, internalAllowed []string, re signature.NewPRReject(), } for _, reg := range internalAllowed { + if clusterScopePolicies[reg] != nil { + // do not set "insecureAcceptAnything" for the reg if this allowed reg is set in the clusterimagepolicy + continue + } transportScopes[reg] = signature.PolicyRequirements{ signature.NewPRInsecureAcceptAnything(), } @@ -531,15 +551,21 @@ func updatePolicyJSON(data []byte, internalBlocked, internalAllowed []string, re } } - policyObj.Transports["docker"] = transportScopes - // The “atomic” policy is the same as the “docker” policy, but “atomic” does not support three or more - // scope segments, so filter those scopes out. - policyObj.Transports["atomic"] = make(signature.PolicyTransportScopes) - for reg, config := range transportScopes { - if strings.Count(reg, "/") >= 3 { - continue + for scope, requirement := range clusterScopePolicies { + transportScopes[scope] = append(transportScopes[scope], requirement...) + } + + if len(transportScopes) > 0 { + policyObj.Transports["docker"] = transportScopes + // The “atomic” policy is the same as the “docker” policy, but “atomic” does not support three or more + // scope segments, so filter those scopes out. + policyObj.Transports["atomic"] = make(signature.PolicyTransportScopes) + for reg, config := range transportScopes { + if strings.Count(reg, "/") >= 3 { + continue + } + policyObj.Transports["atomic"][reg] = config } - policyObj.Transports["atomic"][reg] = config } policyJSON, err := json.Marshal(policyObj) @@ -812,3 +838,119 @@ func convertICSPToIDMS(icsp *apioperatorsv1alpha1.ImageContentSourcePolicy) *api }, } } + +func ownerReferenceImageConfig(imageConfig *apicfgv1.Image) metav1.OwnerReference { + return metav1.OwnerReference{ + APIVersion: apicfgv1.SchemeGroupVersion.String(), + Kind: "Image", + Name: imageConfig.Name, + UID: imageConfig.UID, + } +} + +func policyItemFromSpec(policy apicfgv1alpha1.Policy) (signature.PolicyRequirement, error) { + var ( + sigstorePolicyRequirement signature.PolicyRequirement + signedIdentity signature.PolicyReferenceMatch + signedOptions []signature.PRSigstoreSignedOption + err error + ) + switch policy.RootOfTrust.PolicyType { + case apicfgv1alpha1.PublicKeyRootOfTrust: + signedOptions = append(signedOptions, signature.PRSigstoreSignedWithKeyData(policy.RootOfTrust.PublicKey.KeyData)) + if len(policy.RootOfTrust.PublicKey.RekorKeyData) > 0 { + signedOptions = append(signedOptions, signature.PRSigstoreSignedWithRekorPublicKeyData(policy.RootOfTrust.PublicKey.RekorKeyData)) + } + case apicfgv1alpha1.FulcioCAWithRekorRootOfTrust: + fulcioOptions := []signature.PRSigstoreSignedFulcioOption{} + fulcioOptions = append(fulcioOptions, signature.PRSigstoreSignedFulcioWithCAData(policy.RootOfTrust.FulcioCAWithRekor.FulcioCAData), + signature.PRSigstoreSignedFulcioWithOIDCIssuer(policy.RootOfTrust.FulcioCAWithRekor.FulcioSubject.OIDCIssuer), + signature.PRSigstoreSignedFulcioWithSubjectEmail(policy.RootOfTrust.FulcioCAWithRekor.FulcioSubject.SignedEmail)) + + prSigstoreSignedFulcio, err := signature.NewPRSigstoreSignedFulcio(fulcioOptions...) + if err != nil { + return nil, err + } + signedOptions = append(signedOptions, signature.PRSigstoreSignedWithFulcio(prSigstoreSignedFulcio), signature.PRSigstoreSignedWithRekorPublicKeyData(policy.RootOfTrust.FulcioCAWithRekor.RekorKeyData)) + } + + switch policy.SignedIdentity.MatchPolicy { + case apicfgv1alpha1.IdentityMatchPolicyRemapIdentity: + identity, err := signature.NewPRMRemapIdentity(string(policy.SignedIdentity.PolicyMatchRemapIdentity.Prefix), string(policy.SignedIdentity.PolicyMatchRemapIdentity.SignedPrefix)) + if err != nil { + return nil, fmt.Errorf("error getting signedIdentity for %s: %v", apicfgv1alpha1.IdentityMatchPolicyRemapIdentity, err) + } + signedIdentity = identity + case apicfgv1alpha1.IdentityMatchPolicyExactRepository: + identity, err := signature.NewPRMExactRepository(string(policy.SignedIdentity.PolicyMatchExactRepository.Repository)) + if err != nil { + return nil, fmt.Errorf("error getting signedIdentity for %s: %v", apicfgv1alpha1.IdentityMatchPolicyExactRepository, err) + } + signedIdentity = identity + case apicfgv1alpha1.IdentityMatchPolicyMatchRepository: + signedIdentity = signature.NewPRMMatchRepository() + case apicfgv1alpha1.IdentityMatchPolicyMatchRepoDigestOrExact, "": + signedIdentity = signature.NewPRMMatchRepoDigestOrExact() + default: + return nil, fmt.Errorf("unknown signedIdentity match policy: %s", policy.SignedIdentity.MatchPolicy) + } + + signedOptions = append(signedOptions, signature.PRSigstoreSignedWithSignedIdentity(signedIdentity)) + + if sigstorePolicyRequirement, err = signature.NewPRSigstoreSigned(signedOptions...); err != nil { + return nil, err + } + + return sigstorePolicyRequirement, nil +} + +func validateClusterImagePolicyWithAllowedBlockedRegistries(clusterScopePolicies map[string]signature.PolicyRequirements, allowedRegs, policyBlocked []string) error { + if len(allowedRegs) == 0 && len(policyBlocked) == 0 { + return nil + } + + for scope := range clusterScopePolicies { + scopeNestedInAllowed := false + for _, reg := range allowedRegs { + if runtimeutils.ScopeIsNestedInsideScope(scope, reg) { + scopeNestedInAllowed = true + break + } + } + if len(allowedRegs) > 0 && !scopeNestedInAllowed { + return fmt.Errorf("clusterimagePolicy configured for the scope %s is not nested inside allowedRegistries", scope) + } + } + + for scope := range clusterScopePolicies { + for _, reg := range policyBlocked { + if runtimeutils.ScopeIsNestedInsideScope(scope, reg) { + return fmt.Errorf("clusterimagePolicy configured for the scope %s is nested inside blockedRegistries", scope) + } + + } + } + return nil +} + +func generateSigstoreRegistriesdConfig(clusterScopePolicies map[string]signature.PolicyRequirements) ([]byte, error) { + if len(clusterScopePolicies) == 0 { + return nil, nil + } + + registriesDockerConfig := make(map[string]dockerConfig) + sigstoreAttachment := dockerConfig{ + UseSigstoreAttachments: true, + } + for scope := range clusterScopePolicies { + registriesDockerConfig[scope] = sigstoreAttachment + } + + registriesConfig := ®istriesConfig{} + registriesConfig.Docker = registriesDockerConfig + data, err := yaml.Marshal(registriesConfig) + if err != nil { + return nil, fmt.Errorf("error marshalling regisres configuration for sigstore (cluster)imagepolicies: %w", err) + } + return data, nil +} diff --git a/pkg/controller/container-runtime-config/helpers_test.go b/pkg/controller/container-runtime-config/helpers_test.go index 50434c03a5..8b72859db6 100644 --- a/pkg/controller/container-runtime-config/helpers_test.go +++ b/pkg/controller/container-runtime-config/helpers_test.go @@ -2,8 +2,10 @@ package containerruntimeconfig import ( "bytes" + b64 "encoding/base64" "encoding/json" "errors" + "fmt" "os" "reflect" "testing" @@ -14,6 +16,7 @@ import ( "github.com/containers/image/v5/types" storageconfig "github.com/containers/storage/pkg/config" apicfgv1 "github.com/openshift/api/config/v1" + apicfgv1alpha1 "github.com/openshift/api/config/v1alpha1" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" "github.com/stretchr/testify/assert" @@ -463,7 +466,73 @@ func TestUpdateRegistriesConfig(t *testing.T) { } } +func clusterImagePolicyTestCRs() map[string]apicfgv1alpha1.ClusterImagePolicy { + testFulcioData, _ := b64.StdEncoding.DecodeString("dGVzdC1jYS1kYXRhLWRhdGE=") + testRekorKeyData, _ := b64.StdEncoding.DecodeString("dGVzdC1yZWtvci1rZXktZGF0YQ==") + testKeyData, _ := b64.StdEncoding.DecodeString("dGVzdC1rZXktZGF0YQ==") + + testClusterImagePolicyCRs := map[string]apicfgv1alpha1.ClusterImagePolicy{ + "test-cr0": { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cr0", + }, + Spec: apicfgv1alpha1.ClusterImagePolicySpec{ + Scopes: []apicfgv1alpha1.ImageScope{"test0.com"}, + Policy: apicfgv1alpha1.Policy{ + RootOfTrust: apicfgv1alpha1.PolicyRootOfTrust{ + PolicyType: apicfgv1alpha1.FulcioCAWithRekorRootOfTrust, + FulcioCAWithRekor: &apicfgv1alpha1.FulcioCAWithRekor{ + FulcioCAData: testFulcioData, + RekorKeyData: testRekorKeyData, + FulcioSubject: apicfgv1alpha1.PolicyFulcioSubject{ + OIDCIssuer: "https://OIDC.example.com", + SignedEmail: "test-user@example.com", + }, + }, + }, + SignedIdentity: apicfgv1alpha1.PolicyIdentity{ + MatchPolicy: apicfgv1alpha1.IdentityMatchPolicyRemapIdentity, + PolicyMatchRemapIdentity: &apicfgv1alpha1.PolicyMatchRemapIdentity{ + Prefix: "test-remap-prefix", + SignedPrefix: "test-remap-signed-prefix", + }, + }, + }, + }, + }, + "test-cr1": { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cr1", + }, + Spec: apicfgv1alpha1.ClusterImagePolicySpec{ + Scopes: []apicfgv1alpha1.ImageScope{"test0.com", "test1.com"}, + Policy: apicfgv1alpha1.Policy{ + RootOfTrust: apicfgv1alpha1.PolicyRootOfTrust{ + PolicyType: apicfgv1alpha1.PublicKeyRootOfTrust, + PublicKey: &apicfgv1alpha1.PublicKey{ + KeyData: testKeyData, + RekorKeyData: testRekorKeyData, + }, + }, + SignedIdentity: apicfgv1alpha1.PolicyIdentity{ + MatchPolicy: apicfgv1alpha1.IdentityMatchPolicyRemapIdentity, + PolicyMatchRemapIdentity: &apicfgv1alpha1.PolicyMatchRemapIdentity{ + Prefix: "test-remap-prefix", + SignedPrefix: "test-remap-signed-prefix", + }, + }, + }, + }, + }, + } + return testClusterImagePolicyCRs +} + func TestUpdatePolicyJSON(t *testing.T) { + testClusterImagePolicyCR := clusterImagePolicyTestCRs()["test-cr0"] + expectSigRequirement, policyerr := policyItemFromSpec(testClusterImagePolicyCR.Spec.Policy) + require.NoError(t, policyerr) + templateConfig := signature.Policy{ Default: signature.PolicyRequirements{signature.NewPRInsecureAcceptAnything()}, Transports: map[string]signature.PolicyTransportScopes{ @@ -478,10 +547,11 @@ func TestUpdatePolicyJSON(t *testing.T) { templateBytes := buf.Bytes() tests := []struct { - name string - allowed, blocked []string - errorExpected bool - want signature.Policy + name string + allowed, blocked []string + clusterimagepolicy *apicfgv1alpha1.ClusterImagePolicy + errorExpected bool + want signature.Policy }{ { name: "unchanged", @@ -626,10 +696,42 @@ func TestUpdatePolicyJSON(t *testing.T) { }, errorExpected: false, }, + { + name: "add ClusterImagePolicy CR to policy json", + allowed: []string{"allow.io", "test0.com"}, + clusterimagepolicy: &testClusterImagePolicyCR, + want: signature.Policy{ + Default: signature.PolicyRequirements{signature.NewPRReject()}, + Transports: map[string]signature.PolicyTransportScopes{ + "atomic": map[string]signature.PolicyRequirements{ + "allow.io": {signature.NewPRInsecureAcceptAnything()}, + "test0.com": {expectSigRequirement}, + }, + "docker": map[string]signature.PolicyRequirements{ + "allow.io": {signature.NewPRInsecureAcceptAnything()}, + "test0.com": {expectSigRequirement}, + }, + "docker-daemon": map[string]signature.PolicyRequirements{ + "": {signature.NewPRInsecureAcceptAnything()}, + }, + }, + }, + errorExpected: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := updatePolicyJSON(templateBytes, tt.blocked, tt.allowed, "release-reg.io/image/release") + + var ( + clusterImagePolicies map[string]signature.PolicyRequirements + policyerr error + ) + + if tt.clusterimagepolicy != nil { + clusterImagePolicies, policyerr = getValidScopePolicies([]*apicfgv1alpha1.ClusterImagePolicy{tt.clusterimagepolicy}, "release-reg.io/image/release", nil) + require.NoError(t, policyerr) + } + got, err := updatePolicyJSON(templateBytes, tt.blocked, tt.allowed, "release-reg.io/image/release", clusterImagePolicies) if err == nil && tt.errorExpected { t.Errorf("updatePolicyJSON() error = %v", err) return @@ -1105,3 +1207,232 @@ func TestUpdateStorageConfig(t *testing.T) { } } } + +func TestGetValidScopePolicies(t *testing.T) { + type testcase struct { + name string + clusterImagePolicyCRs []*apicfgv1alpha1.ClusterImagePolicy + releaseImg string + expectedScopePolicies map[string]signature.PolicyRequirements + errorExpected bool + } + + testCR0 := clusterImagePolicyTestCRs()["test-cr0"] + testCR1 := clusterImagePolicyTestCRs()["test-cr1"] + policyreq0, err := policyItemFromSpec(testCR0.Spec.Policy) + require.NoError(t, err) + policyreq1, err := policyItemFromSpec(testCR1.Spec.Policy) + require.NoError(t, err) + + tests := []testcase{ + { + name: "cluster CRs contain the same scope", + clusterImagePolicyCRs: []*apicfgv1alpha1.ClusterImagePolicy{&testCR0, &testCR1}, + releaseImg: "release-reg.io/image/release", + expectedScopePolicies: map[string]signature.PolicyRequirements{ + "test0.com": {policyreq0, policyreq1}, + "test1.com": {policyreq1}, + }, + errorExpected: false, + }, + { + name: "clusterimagepolicy CRs has scope has super scope of release image", + clusterImagePolicyCRs: []*apicfgv1alpha1.ClusterImagePolicy{&testCR0, &testCR1}, + releaseImg: "test1.com/image/release", + expectedScopePolicies: map[string]signature.PolicyRequirements{ + "test0.com": {policyreq0, policyreq1}, + }, + errorExpected: false, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + gotScopePolicies, err := getValidScopePolicies(test.clusterImagePolicyCRs, test.releaseImg, nil) + fmt.Println(err) + require.Equal(t, test.errorExpected, err != nil) + if !test.errorExpected { + require.Equal(t, test.expectedScopePolicies, gotScopePolicies) + } + }) + } +} + +func TestGenerateSigstoreRegistriesConfig(t *testing.T) { + testClusterImagePolicyCR0 := clusterImagePolicyTestCRs()["test-cr0"] + testClusterImagePolicyCR1 := clusterImagePolicyTestCRs()["test-cr1"] + + expectSigstoreRegistriesConfig := []byte( + `docker: + test0.com: + use-sigstore-attachments: true + test1.com: + use-sigstore-attachments: true +`) + + clusterScopePolicies, err := getValidScopePolicies([]*apicfgv1alpha1.ClusterImagePolicy{&testClusterImagePolicyCR0, &testClusterImagePolicyCR1}, "release-reg.io/image/release", nil) + require.NoError(t, err) + got, err := generateSigstoreRegistriesdConfig(clusterScopePolicies) + require.NoError(t, err) + require.Equal(t, string(expectSigstoreRegistriesConfig), string(got)) +} + +func TestGeneratePolicyJSON(t *testing.T) { + + testImagePolicyCR0 := clusterImagePolicyTestCRs()["test-cr0"] + + expectClusterPolicy := []byte(` + { + "default": [ + { + "type": "insecureAcceptAnything" + } + ], + "transports": { + "atomic": { + "test0.com": [ + { + "type": "sigstoreSigned", + "fulcio": { + "caData": "dGVzdC1jYS1kYXRhLWRhdGE=", + "oidcIssuer": "https://OIDC.example.com", + "subjectEmail": "test-user@example.com" + }, + "rekorPublicKeyData": "dGVzdC1yZWtvci1rZXktZGF0YQ==", + "signedIdentity": { + "type": "remapIdentity", + "prefix": "test-remap-prefix", + "signedPrefix": "test-remap-signed-prefix" + } + } + ] + }, + "docker": { + "test0.com": [ + { + "type": "sigstoreSigned", + "fulcio": { + "caData": "dGVzdC1jYS1kYXRhLWRhdGE=", + "oidcIssuer": "https://OIDC.example.com", + "subjectEmail": "test-user@example.com" + }, + "rekorPublicKeyData": "dGVzdC1yZWtvci1rZXktZGF0YQ==", + "signedIdentity": { + "type": "remapIdentity", + "prefix": "test-remap-prefix", + "signedPrefix": "test-remap-signed-prefix" + } + } + ] + }, + "docker-daemon": { + "": [ + { + "type": "insecureAcceptAnything" + } + ] + } + } + } + `) + + clusterScopePolicies, err := getValidScopePolicies([]*apicfgv1alpha1.ClusterImagePolicy{&testImagePolicyCR0}, "release-reg.io/image/release", nil) + require.NoError(t, err) + + templateConfig := signature.Policy{ + Default: signature.PolicyRequirements{signature.NewPRInsecureAcceptAnything()}, + Transports: map[string]signature.PolicyTransportScopes{ + "docker-daemon": map[string]signature.PolicyRequirements{ + "": {signature.NewPRInsecureAcceptAnything()}, + }, + }, + } + buf := bytes.Buffer{} + err = json.NewEncoder(&buf).Encode(templateConfig) + require.NoError(t, err) + templateBytes := buf.Bytes() + + baseData, err := updatePolicyJSON(templateBytes, []string{}, []string{}, "release-reg.io/image/release", clusterScopePolicies) + require.NoError(t, err) + require.JSONEq(t, string(expectClusterPolicy), string(baseData)) + +} + +func TestValidateClusterImagePolicyWithAllowedBlockedRegistries(t *testing.T) { + tests := []struct { + name string + allowed []string + blocked []string + clusterScopePolicies map[string]signature.PolicyRequirements + errorExpected bool + }{ + { + name: "allowed and blocked registries are not set", + clusterScopePolicies: map[string]signature.PolicyRequirements{ + "example.com": {}, + "test.registries.com": {}, + }, + errorExpected: false, + }, + { + name: "success test allowed registries set", + allowed: []string{ + "example.com", + "allowed.io/namespace1", + }, + clusterScopePolicies: map[string]signature.PolicyRequirements{ + "allowed.io/namespace1/policysigned": {}, + "example.com": {}, + }, + errorExpected: false, + }, + { + name: "failure test allowed registries set", + allowed: []string{ + "allowed.io/namespace1/namespace2", + }, + clusterScopePolicies: map[string]signature.PolicyRequirements{ + "allowed.io/policysigned": {}, + }, + errorExpected: true, + }, + { + name: "success test blocked registries set", + blocked: []string{ + "blocked.io/namespace1/namespace2", + }, + clusterScopePolicies: map[string]signature.PolicyRequirements{ + "allowed.io/policysigned": {}, + "blocked.io": {}, + }, + errorExpected: false, + }, + { + name: "failure test blocked registries set", + blocked: []string{ + "blocked.io", + "blocker-reg.com/namespace1", + }, + clusterScopePolicies: map[string]signature.PolicyRequirements{ + "blocker-reg.com/namespace1/policysigned": {}, + "blocked.io": {}, + }, + errorExpected: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateClusterImagePolicyWithAllowedBlockedRegistries(tt.clusterScopePolicies, tt.allowed, tt.blocked) + if err == nil && tt.errorExpected { + t.Errorf("validateClusterImagePolicyWithAllowedBlockedRegistries() error = %v", err) + return + } + if err != nil { + if tt.errorExpected { + return + } + t.Errorf("validateClusterImagePolicyWithAllowedBlockedRegistries() error = %v", err) + return + } + }) + } +} diff --git a/pkg/daemon/constants/constants.go b/pkg/daemon/constants/constants.go index e89cc6ab8f..6619cbec53 100644 --- a/pkg/daemon/constants/constants.go +++ b/pkg/daemon/constants/constants.go @@ -94,6 +94,9 @@ const ( // changes to registries.conf will cause a crio reload and require extra logic about whether to drain ContainerRegistryConfPath = "/etc/containers/registries.conf" + // changes to registries.d will cause a crio reload + SigstoreRegistriesConfigDir = "/etc/containers/registries.d" + // SSH Keys for user "core" will only be written at /home/core/.ssh CoreUserSSHPath = "/home/" + CoreUserName + "/.ssh" diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index b3a68e9865..f83d7d9f17 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -452,6 +452,9 @@ func calculatePostConfigChangeActionFromFileDiffs(diffFileSet []string) (actions filesPostConfigChangeActionRestartCrio := []string{ "/etc/pki/ca-trust/source/anchors/openshift-config-user-ca-bundle.crt", } + dirsPostConfigChangeActionReloadCrio := []string{ + constants.SigstoreRegistriesConfigDir, + } actions = []string{postConfigChangeActionNone} for _, path := range diffFileSet { @@ -461,6 +464,8 @@ func calculatePostConfigChangeActionFromFileDiffs(diffFileSet []string) (actions actions = []string{postConfigChangeActionReloadCrio} } else if ctrlcommon.InSlice(path, filesPostConfigChangeActionRestartCrio) { actions = []string{postConfigChangeActionRestartCrio} + } else if ctrlcommon.InSlice(filepath.Dir(path), dirsPostConfigChangeActionReloadCrio) { + actions = []string{postConfigChangeActionReloadCrio} } else { actions = []string{postConfigChangeActionReboot} return diff --git a/test/e2e-bootstrap/bootstrap_test.go b/test/e2e-bootstrap/bootstrap_test.go index 93c21ef7ff..113f20b759 100644 --- a/test/e2e-bootstrap/bootstrap_test.go +++ b/test/e2e-bootstrap/bootstrap_test.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/require" configv1 "github.com/openshift/api/config/v1" + configv1alpha1 "github.com/openshift/api/config/v1alpha1" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" featuregatescontroller "github.com/openshift/cluster-config-operator/pkg/operator/featuregates" @@ -65,6 +66,7 @@ func TestE2EBootstrap(t *testing.T) { testEnv := framework.NewTestEnv(t) configv1.Install(scheme.Scheme) + configv1alpha1.Install(scheme.Scheme) mcfgv1.Install(scheme.Scheme) apioperatorsv1alpha1.Install(scheme.Scheme) @@ -485,6 +487,7 @@ func createControllers(ctx *ctrlcommon.ControllerContext) []ctrlcommon.Controlle ctx.ConfigInformerFactory.Config().V1().Images(), ctx.ConfigInformerFactory.Config().V1().ImageDigestMirrorSets(), ctx.ConfigInformerFactory.Config().V1().ImageTagMirrorSets(), + ctx.ConfigInformerFactory, ctx.OperatorInformerFactory.Operator().V1alpha1().ImageContentSourcePolicies(), ctx.ConfigInformerFactory.Config().V1().ClusterVersions(), ctx.ClientBuilder.KubeClientOrDie("container-runtime-config-controller"), @@ -614,7 +617,7 @@ func loadBaseTestManifests(t *testing.T) []runtime.Object { func loadRawManifests(t *testing.T, rawObjs [][]byte) []runtime.Object { codecFactory := serializer.NewCodecFactory(scheme.Scheme) - decoder := codecFactory.UniversalDecoder(corev1GroupVersion, mcfgv1.GroupVersion, apioperatorsv1alpha1.GroupVersion, configv1.GroupVersion) + decoder := codecFactory.UniversalDecoder(corev1GroupVersion, mcfgv1.GroupVersion, apioperatorsv1alpha1.GroupVersion, configv1alpha1.GroupVersion, configv1.GroupVersion) objs := []runtime.Object{} for _, raw := range rawObjs {