diff --git a/internal/dataplane/inventory.go b/internal/dataplane/inventory.go index e81bf5dde2..1076790005 100644 --- a/internal/dataplane/inventory.go +++ b/internal/dataplane/inventory.go @@ -171,9 +171,6 @@ func GenerateNodeSetInventory(ctx context.Context, helper *helper.Helper, nodeSetGroup.Vars["edpm_container_signature_verification"] = true nodeSetGroup.Vars["edpm_container_signature_registry_mappings"] = sigstorePolicy.RegistryMappings nodeSetGroup.Vars["edpm_container_signature_cosign_key_data"] = sigstorePolicy.CosignKeyData - if sigstorePolicy.SignedPrefix != "" { - nodeSetGroup.Vars["edpm_container_signature_signed_prefix"] = sigstorePolicy.SignedPrefix - } } else { helper.GetLogger().Info("No matching ClusterImagePolicy found; skipping sigstore verification") } diff --git a/internal/dataplane/util/image_registry.go b/internal/dataplane/util/image_registry.go index 5de520075b..193421f457 100644 --- a/internal/dataplane/util/image_registry.go +++ b/internal/dataplane/util/image_registry.go @@ -227,19 +227,17 @@ func getMachineConfig(ctx context.Context, helper *helper.Helper) (mc.MachineCon return masterMachineConfig, nil } -// RegistryMapping pairs a mirror registry with its upstream source from IDMS/ICSP. +// RegistryMapping pairs a mirror with its upstream source. type RegistryMapping struct { - Mirror string `json:"mirror"` - Source string `json:"source"` + Mirror string `json:"mirror"` + Source string `json:"source"` + SignedPrefix string `json:"signedPrefix,omitempty"` } -// SigstorePolicyInfo contains the EDPM-relevant parts of a ClusterImagePolicy. -// A single RemapIdentity signedPrefix covers all mirrors under the same registry -// root — the container runtime replaces only the prefix, preserving namespace paths. +// SigstorePolicyInfo contains the EDPM sigstore settings. type SigstorePolicyInfo struct { RegistryMappings []RegistryMapping CosignKeyData string - SignedPrefix string } const ( @@ -319,6 +317,24 @@ func listClusterImagePolicies( return policyList, nil } +func collectMatchedMirrorScopes(policyScopes []string, mirrorScopes []string) []string { + matchedMirrorScopes := map[string]struct{}{} + for _, scope := range policyScopes { + policyScope := normalizeImageScope(scope) + if policyScope == "" { + continue + } + + for _, mirrorScope := range mirrorScopes { + if clusterImagePolicyScopeMatchesMirror(policyScope, mirrorScope) { + matchedMirrorScopes[mirrorScope] = struct{}{} + } + } + } + + return sortedSetKeys(matchedMirrorScopes) +} + // GetSigstoreImagePolicy checks if OCP has a ClusterImagePolicy configured // with sigstore signature verification for one of the mirror registries in use. // sourceByMirror maps each mirror scope to its upstream source registry (from IDMS/ICSP). @@ -345,8 +361,25 @@ func GetSigstoreImagePolicy(ctx context.Context, helper *helper.Helper, mirrorSc return nil, nil } - var matches []string - var match *SigstorePolicyInfo + type policyMatch struct { + name string + keyData string + signedPrefix string + } + + normalizedMirrors := make([]string, 0, len(mirrorScopes)) + sourceByNormalizedMirror := make(map[string]string, len(sourceByMirror)) + for _, mirrorScope := range mirrorScopes { + normalizedMirror := normalizeImageScope(mirrorScope) + if normalizedMirror == "" { + continue + } + normalizedMirrors = append(normalizedMirrors, normalizedMirror) + sourceByNormalizedMirror[normalizedMirror] = sourceByMirror[mirrorScope] + } + sort.Strings(normalizedMirrors) + + matchByMirror := map[string]policyMatch{} for _, policy := range policyList.Items { if policy.GetName() == "openshift" { @@ -392,47 +425,57 @@ func GetSigstoreImagePolicy(ctx context.Context, helper *helper.Helper, mirrorSc } } - matchedMirrorScopes := map[string]struct{}{} - for _, scope := range scopes { - policyScope := normalizeImageScope(scope) - if policyScope == "" { - continue - } + matchedMirrors := collectMatchedMirrorScopes(scopes, normalizedMirrors) + if len(matchedMirrors) == 0 { + continue + } - for _, mirrorScope := range mirrorScopes { - if clusterImagePolicyScopeMatchesMirror(policyScope, mirrorScope) { - matchedMirrorScopes[normalizeImageScope(mirrorScope)] = struct{}{} - } - } + match := policyMatch{ + name: policy.GetName(), + keyData: keyData, + signedPrefix: signedPrefix, } - if len(matchedMirrorScopes) == 0 { - continue + for _, mirror := range matchedMirrors { + if existing, found := matchByMirror[mirror]; found { + return nil, fmt.Errorf( + "mirror scope %s matched multiple ClusterImagePolicies: %s, %s", + mirror, existing.name, policy.GetName(), + ) + } + matchByMirror[mirror] = match } + } - sortedMirrors := sortedSetKeys(matchedMirrorScopes) + if len(matchByMirror) == 0 { + return nil, nil + } - mappings := make([]RegistryMapping, 0, len(sortedMirrors)) - for _, m := range sortedMirrors { - mappings = append(mappings, RegistryMapping{ - Mirror: m, - Source: sourceByMirror[m], - }) - } + sortedMirrors := make([]string, 0, len(matchByMirror)) + for mirror := range matchByMirror { + sortedMirrors = append(sortedMirrors, mirror) + } + sort.Strings(sortedMirrors) - matches = append(matches, fmt.Sprintf("%s (%s)", policy.GetName(), strings.Join(sortedMirrors, ", "))) - match = &SigstorePolicyInfo{ - RegistryMappings: mappings, - CosignKeyData: keyData, - SignedPrefix: signedPrefix, - } + firstMatch := matchByMirror[sortedMirrors[0]] + match := &SigstorePolicyInfo{ + RegistryMappings: make([]RegistryMapping, 0, len(sortedMirrors)), + CosignKeyData: firstMatch.keyData, } - if len(matches) > 1 { - sort.Strings(matches) - return nil, fmt.Errorf( - "expected exactly one ClusterImagePolicy matching mirror registries, found %d: %s", - len(matches), strings.Join(matches, ", "), - ) + for _, mirror := range sortedMirrors { + mirrorMatch := matchByMirror[mirror] + if match.CosignKeyData != mirrorMatch.keyData { + return nil, fmt.Errorf( + "matched ClusterImagePolicies use different cosign key data and cannot be combined: %s, %s", + firstMatch.name, mirrorMatch.name, + ) + } + + match.RegistryMappings = append(match.RegistryMappings, RegistryMapping{ + Mirror: mirror, + Source: sourceByNormalizedMirror[mirror], + SignedPrefix: mirrorMatch.signedPrefix, + }) } return match, nil diff --git a/internal/dataplane/util/image_registry_test.go b/internal/dataplane/util/image_registry_test.go index a1bd93c493..c5c902410a 100644 --- a/internal/dataplane/util/image_registry_test.go +++ b/internal/dataplane/util/image_registry_test.go @@ -778,10 +778,9 @@ func TestGetSigstoreImagePolicy_WithRemapIdentity(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) g.Expect(result).ToNot(BeNil()) g.Expect(result.RegistryMappings).To(Equal([]RegistryMapping{ - {Mirror: "local-registry.example.com:5000", Source: "registry.example.com/vendor"}, + {Mirror: "local-registry.example.com:5000", Source: "registry.example.com/vendor", SignedPrefix: "registry.example.com/vendor"}, })) g.Expect(result.CosignKeyData).To(Equal(base64.StdEncoding.EncodeToString([]byte("test-public-key")))) - g.Expect(result.SignedPrefix).To(Equal("registry.example.com/vendor")) } func TestGetSigstoreImagePolicy(t *testing.T) { @@ -806,7 +805,6 @@ func TestGetSigstoreImagePolicy(t *testing.T) { {Mirror: "local-registry.example.com:5000"}, })) g.Expect(result.CosignKeyData).To(Equal(base64.StdEncoding.EncodeToString([]byte("test-public-key")))) - g.Expect(result.SignedPrefix).To(BeEmpty()) } func TestGetSigstoreImagePolicy_ReturnsAllMatchingMirrorScopes(t *testing.T) { @@ -886,7 +884,83 @@ func TestGetSigstoreImagePolicy_ReturnsErrorForAmbiguousPolicies(t *testing.T) { result, err := GetSigstoreImagePolicy(ctx, h, []string{"mirror.example.com:5000/openstack-k8s-operators"}, nil) g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).To(ContainSubstring("expected exactly one ClusterImagePolicy matching mirror registries")) + g.Expect(err.Error()).To(ContainSubstring("mirror scope mirror.example.com:5000/openstack-k8s-operators matched multiple ClusterImagePolicies")) + g.Expect(result).To(BeNil()) +} + +func TestGetSigstoreImagePolicy_AllowsDifferentPoliciesPerMirrorScope(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + + policy1 := newSigstorePolicy( + "v1alpha1", + "policy-rhoso", + []string{"mirror.example.com:5000/rhoso"}, + "shared-key", + "RemapIdentity", + "registry.redhat.io/rhoso", + ) + policy2 := newSigstorePolicy( + "v1alpha1", + "policy-rhoso-operators", + []string{"mirror.example.com:5000/rhoso-operators"}, + "shared-key", + "RemapIdentity", + "registry.redhat.io/rhoso-operators", + ) + + h := setupTestHelper(true, newClusterImagePolicyCRD("v1alpha1"), policy1, policy2) + + sourceByMirror := map[string]string{ + "mirror.example.com:5000/rhoso": "registry.redhat.io/rhoso", + "mirror.example.com:5000/rhoso-operators": "registry.redhat.io/rhoso-operators", + } + result, err := GetSigstoreImagePolicy(ctx, h, []string{ + "mirror.example.com:5000/rhoso", + "mirror.example.com:5000/rhoso-operators", + }, sourceByMirror) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result).ToNot(BeNil()) + g.Expect(result.RegistryMappings).To(Equal([]RegistryMapping{ + {Mirror: "mirror.example.com:5000/rhoso", Source: "registry.redhat.io/rhoso", SignedPrefix: "registry.redhat.io/rhoso"}, + {Mirror: "mirror.example.com:5000/rhoso-operators", Source: "registry.redhat.io/rhoso-operators", SignedPrefix: "registry.redhat.io/rhoso-operators"}, + })) + g.Expect(result.CosignKeyData).To(Equal(base64.StdEncoding.EncodeToString([]byte("shared-key")))) +} + +func TestGetSigstoreImagePolicy_ReturnsErrorForDifferentKeysAcrossPolicies(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + + policy1 := newSigstorePolicy( + "v1alpha1", + "policy-rhoso", + []string{"mirror.example.com:5000/rhoso"}, + "key-one", + "RemapIdentity", + "registry.redhat.io/rhoso", + ) + policy2 := newSigstorePolicy( + "v1alpha1", + "policy-ubi9", + []string{"mirror.example.com:5000/ubi9"}, + "key-two", + "RemapIdentity", + "registry.redhat.io/ubi9", + ) + + h := setupTestHelper(true, newClusterImagePolicyCRD("v1alpha1"), policy1, policy2) + + sourceByMirror := map[string]string{ + "mirror.example.com:5000/rhoso": "registry.redhat.io/rhoso", + "mirror.example.com:5000/ubi9": "registry.redhat.io/ubi9", + } + result, err := GetSigstoreImagePolicy(ctx, h, []string{ + "mirror.example.com:5000/rhoso", + "mirror.example.com:5000/ubi9", + }, sourceByMirror) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("different cosign key data")) g.Expect(result).To(BeNil()) }