Merge pull request #714 from danielerez/ocpbugs-86794-disable-sigstore-registries

OCPBUGS-86838: Support disableSigstoreRegistries list for mirroring

Author: danielerez

docs/appliance-config.md

@@ -23,7 +23,7 @@
 | enableDefaultSources       | false                          | Yes      | bool    | Enable all default CatalogSources (on openshift-marketplace namespace). Should be disabled for disconnected environments.                                                                                                                                                                                                                                                                                     |
 | enableFips                 | false                          | Yes      | bool    | Enable FIPS mode for the cluster. Note: 'fips' should be enabled also in install-config.yaml. |
 | enableInteractiveFlow                 | false                          | Yes      | bool    | Enable the interactive installation flow. Should be enabled to provide cluster configuration through the web UI (i.e. instead of using a config-image). |
-| disableSigstoreForAdditionalImages | false                    | Yes      | bool    | Disable sigstore attachments for all image domains listed under `additionalImages` (useful when source registries do not publish `.sig` attachment manifests). **Note:** this is a workaround for images that do not publish OCI `.sig` manifests (failing oc-mirror v2). |
+| disableSigstoreRegistries          |                                | Yes      | array   | Registry hosts for which oc-mirror should not fetch sigstore `.sig` attachments (workaround when registries do not publish OCI `.sig` manifests). Example: `registry.connect.redhat.com` for certified operator bundles. Hosts that already have `/etc/containers/registries.d/<host>.yaml` in the build environment are skipped. |
 | additionalImages           |                                | Yes      | array   | Additional images to be included in the appliance disk image.                                                                                                                                                                                                                                                                                                                                                 |
 | blockedImages           |                                | Yes      | array   | Images to avoid including in the appliance disk image (by name or regular expression). |
 | operators                  |                                | Yes      | array   | Operators to be included in the appliance disk image. See examples in https://github.com/openshift/oc-mirror/blob/main/docs/imageset-config-ref.yaml.                                                                                                                                                                                                                                                         |

docs/user-guide.md

@@ -160,13 +160,12 @@ enableInteractiveFlow: enable-interactive-flow
 # Default: false
 # [Optional]
 useDefaultSourceNames: use-default-source-names
-# Disable sigstore attachments for all registries referenced in additionalImages.
-# This can be required for disconnected mirroring when signature attachment manifests
-# are unavailable in source registries.
-# Note: this is a workaround for images that do not publish OCI .sig manifests (failing oc-mirror v2).
-# Default: false
+# Disable sigstore attachments for the listed registry hosts during oc-mirror.
+# Required when source registries do not publish OCI .sig attachment manifests (failing oc-mirror v2).
+# Example: certified operator bundles on registry.connect.redhat.com.
 # [Optional]
-disableSigstoreForAdditionalImages: disable-sigstore-for-additional-images
+# disableSigstoreRegistries:
+#   - registry.connect.redhat.com
 # Additional images to be included in the appliance disk image.
 # [Optional]
 additionalImages:
@@ -292,6 +291,19 @@ Note: for each operator, ensure the name and channel are correct by listing the
 oc-mirror list operators --catalog=registry.redhat.io/redhat/redhat-operator-index:v4.14
 ```
 
+Certified operators that pull bundle images from `registry.connect.redhat.com` may require disabling sigstore attachments during mirroring:
+
+```yaml
+disableSigstoreRegistries:
+  - registry.connect.redhat.com
+operators:
+  - catalog: registry.redhat.io/redhat/redhat-operator-index:v4.19
+    packages:
+      - name: gpu-operator-certified
+        channels:
+          - name: v26.3
+```
+
 #### Install operators in cluster
 
 To automatically install the included operators during cluster installation, add the relevant custom manifests to `${APPLIANCE_ASSETS}/openshift`.

pkg/asset/config/appliance_config.go

@@ -202,13 +202,13 @@ pullSecret: pull-secret
 # [Optional]
 # useDefaultSourceNames: %t
 
-# Disable sigstore attachments for all registries referenced in additionalImages.
-# This can be required for disconnected mirroring when signature attachment manifests
-# are unavailable in source registries.
-# Note: this is a workaround for images that do not publish OCI .sig manifests (failing oc-mirror v2).
-# Default: false
+# Disable sigstore attachments for the listed registry hosts during oc-mirror.
+# Required when source registries do not publish OCI .sig attachment manifests (failing oc-mirror v2).
+# Example: certified operator bundles on registry.connect.redhat.com.
+# Registries that already have configuration under /etc/containers/registries.d/ are skipped.
 # [Optional]
-# disableSigstoreForAdditionalImages: false
+# disableSigstoreRegistries:
+#   - registry.connect.redhat.com
 
 # Additional images to be included in the appliance disk image.
 # [Optional]

pkg/release/sigstore_registries.go

@@ -1,30 +1,53 @@
 package release
 
 import (
+	"os"
+	"path/filepath"
 	"sort"
 	"strings"
 
-	"github.com/go-openapi/swag"
-	"github.com/openshift/appliance/pkg/types"
 )
 
-const (
-	disableSigstoreFilePath = "/etc/containers/registries.d/00-appliance-disable-sigstore.yaml"
-	registriesDDir          = "/etc/containers/registries.d"
-)
+const registriesDDir = "/etc/containers/registries.d"
+
+func isValidRegistryHost(host string) bool {
+	return strings.Contains(host, ".") || host == "localhost"
+}
 
-func additionalImageRegistryHosts(images *[]types.Image) []string {
-	if images == nil {
+func normalizeRegistryHost(entry string) string {
+	entry = strings.TrimSpace(entry)
+	if entry == "" {
+		return ""
+	}
+	if host := imageReferenceRegistryHost(entry); host != "" {
+		if isValidRegistryHost(host) {
+			return host
+		}
+		return ""
+	}
+	host := strings.TrimPrefix(entry, "docker://")
+	if strings.Contains(host, "/") {
+		return ""
+	}
+	if colon := strings.LastIndex(host, ":"); colon > 0 && !strings.Contains(host, "]") {
+		host = host[:colon]
+	}
+	if !isValidRegistryHost(host) {
+		return ""
+	}
+	return host
+}
+
+func disableSigstoreRegistryHosts(registries *[]string) []string {
+	if registries == nil {
 		return nil
 	}
 
 	registryHostsMap := map[string]struct{}{}
-	for _, img := range *images {
-		registryHost := imageReferenceRegistryHost(img.Name)
-		if registryHost == "" {
-			continue
+	for _, entry := range *registries {
+		if host := normalizeRegistryHost(entry); host != "" {
+			registryHostsMap[host] = struct{}{}
 		}
-		registryHostsMap[registryHost] = struct{}{}
 	}
 
 	registryHosts := make([]string, 0, len(registryHostsMap))
@@ -52,6 +75,25 @@ func imageReferenceRegistryHost(name string) string {
 	return ""
 }
 
+func disableSigstoreRegistryConfigPath(host string) string {
+	return filepath.Join(registriesDDir, host+".yaml")
+}
+
+func hostHasRegistriesDConfig(registriesDir, host string, stat func(string) (os.FileInfo, error)) bool {
+	_, err := stat(filepath.Join(registriesDir, host+".yaml"))
+	return err == nil
+}
+
+func filterHostsWithoutExistingRegistriesDConfig(hosts []string, registriesDir string, stat func(string) (os.FileInfo, error)) []string {
+	filtered := make([]string, 0, len(hosts))
+	for _, host := range hosts {
+		if !hostHasRegistriesDConfig(registriesDir, host, stat) {
+			filtered = append(filtered, host)
+		}
+	}
+	return filtered
+}
+
 // buildDisableSigstoreRegistriesConfig creates a containers-registries.d snippet
 // that disables sigstore attachments for the given registry hosts.
 // This is used as a workaround for oc-mirror v2 flows where some registries do not
@@ -72,11 +114,12 @@ func buildDisableSigstoreRegistriesConfig(registryHosts []string) []byte {
 }
 
 func (r *release) disableSigstoreForRelevantRegistries() error {
-	if !swag.BoolValue(r.ApplianceConfig.Config.DisableSigstoreForAdditionalImages) {
+	registryHosts := disableSigstoreRegistryHosts(r.ApplianceConfig.Config.DisableSigstoreRegistries)
+	if len(registryHosts) == 0 {
 		return nil
 	}
 
-	registryHosts := additionalImageRegistryHosts(r.ApplianceConfig.Config.AdditionalImages)
+	registryHosts = filterHostsWithoutExistingRegistriesDConfig(registryHosts, registriesDDir, r.OSInterface.Stat)
 	if len(registryHosts) == 0 {
 		return nil
 	}
@@ -85,5 +128,12 @@ func (r *release) disableSigstoreForRelevantRegistries() error {
 		return err
 	}
 
-	return r.OSInterface.WriteFile(disableSigstoreFilePath, buildDisableSigstoreRegistriesConfig(registryHosts), 0o644)
+	for _, host := range registryHosts {
+		configPath := disableSigstoreRegistryConfigPath(host)
+		if err := r.OSInterface.WriteFile(configPath, buildDisableSigstoreRegistriesConfig([]string{host}), 0o644); err != nil {
+			return err
+		}
+	}
+
+	return nil
 }

pkg/release/sigstore_registries_test.go

@@ -1,10 +1,10 @@
 package release
 
 import (
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
-
-	"github.com/openshift/appliance/pkg/types"
 )
 
 func TestImageReferenceRegistryHost(t *testing.T) {
@@ -29,31 +29,57 @@ func TestImageReferenceRegistryHost(t *testing.T) {
 	}
 }
 
-func TestAdditionalImageRegistryHosts(t *testing.T) {
+func TestNormalizeRegistryHost(t *testing.T) {
+	t.Parallel()
+	cases := []struct {
+		in   string
+		want string
+	}{
+		{"registry.connect.redhat.com", "registry.connect.redhat.com"},
+		{" registry.connect.redhat.com ", "registry.connect.redhat.com"},
+		{"registry.connect.redhat.com:443", "registry.connect.redhat.com"},
+		{"registry.connect.redhat.com/partner/foo:latest", "registry.connect.redhat.com"},
+		{"docker://quay.io/example:latest", "quay.io"},
+		{"", ""},
+		{"invalid/host/extra", ""},
+	}
+	for _, tc := range cases {
+		t.Run(tc.in, func(t *testing.T) {
+			t.Parallel()
+			if got := normalizeRegistryHost(tc.in); got != tc.want {
+				t.Fatalf("normalizeRegistryHost(%q) = %q, want %q", tc.in, got, tc.want)
+			}
+		})
+	}
+}
+
+func TestDisableSigstoreRegistryHosts(t *testing.T) {
 	t.Parallel()
-	images := []types.Image{
-		{Name: "registry.connect.redhat.com/partner/foo@sha256:deadbeef"},
-		{Name: "docker://quay.io/example/sidecar:latest"},
-		{Name: "registry.connect.redhat.com/partner/bar:1.0"},
+	registries := []string{
+		"registry.connect.redhat.com",
+		" registry.connect.redhat.com ",
+		"quay.io",
+		"quay.io/example:latest",
+		"",
 	}
-	got := additionalImageRegistryHosts(&images)
+	got := disableSigstoreRegistryHosts(&registries)
 	want := []string{"quay.io", "registry.connect.redhat.com"}
 	if len(got) != len(want) {
-		t.Fatalf("additionalImageRegistryHosts() returned %d hosts, want %d", len(got), len(want))
+		t.Fatalf("disableSigstoreRegistryHosts() returned %d hosts, want %d", len(got), len(want))
 	}
 	for i := range want {
 		if got[i] != want[i] {
-			t.Fatalf("additionalImageRegistryHosts() host at index %d = %q, want %q", i, got[i], want[i])
+			t.Fatalf("disableSigstoreRegistryHosts() host at index %d = %q, want %q", i, got[i], want[i])
 		}
 	}
 
-	if hosts := additionalImageRegistryHosts(nil); len(hosts) != 0 {
-		t.Fatalf("expected empty list for nil images, got %v", hosts)
+	if hosts := disableSigstoreRegistryHosts(nil); len(hosts) != 0 {
+		t.Fatalf("expected empty list for nil registries, got %v", hosts)
 	}
 
-	var empty []types.Image
-	if hosts := additionalImageRegistryHosts(&empty); len(hosts) != 0 {
-		t.Fatalf("expected empty list for empty images, got %v", hosts)
+	var empty []string
+	if hosts := disableSigstoreRegistryHosts(&empty); len(hosts) != 0 {
+		t.Fatalf("expected empty list for empty registries, got %v", hosts)
 	}
 }
 
@@ -72,3 +98,32 @@ func TestBuildDisableSigstoreRegistriesConfig(t *testing.T) {
 		t.Fatal("expected registry.connect.redhat.com entry in generated config")
 	}
 }
+
+func TestFilterHostsWithoutExistingRegistriesDConfig(t *testing.T) {
+	t.Parallel()
+
+	registriesDir := t.TempDir()
+	if err := os.WriteFile(filepath.Join(registriesDir, "registry.redhat.io.yaml"), []byte("docker:\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	hosts := []string{"registry.redhat.io", "registry.connect.redhat.com", "quay.io"}
+	got := filterHostsWithoutExistingRegistriesDConfig(hosts, registriesDir, os.Stat)
+	want := []string{"registry.connect.redhat.com", "quay.io"}
+	if len(got) != len(want) {
+		t.Fatalf("filterHostsWithoutExistingRegistriesDConfig() = %v, want %v", got, want)
+	}
+	for i := range want {
+		if got[i] != want[i] {
+			t.Fatalf("host at index %d = %q, want %q", i, got[i], want[i])
+		}
+	}
+}
+
+func TestDisableSigstoreRegistryConfigPath(t *testing.T) {
+	t.Parallel()
+	want := filepath.Join(registriesDDir, "registry.connect.redhat.com.yaml")
+	if got := disableSigstoreRegistryConfigPath("registry.connect.redhat.com"); got != want {
+		t.Fatalf("disableSigstoreRegistryConfigPath() = %q, want %q", got, want)
+	}
+}

pkg/types/appliance_config_type.go

@@ -26,7 +26,7 @@ type ApplianceConfig struct {
 	CreatePinnedImageSets              *bool          `json:"createPinnedImageSets"`
 	EnableInteractiveFlow              *bool          `json:"enableInteractiveFlow"`
 	UseDefaultSourceNames              *bool          `json:"useDefaultSourceNames"`
-	DisableSigstoreForAdditionalImages *bool          `json:"disableSigstoreForAdditionalImages,omitempty"`
+	DisableSigstoreRegistries          *[]string      `json:"disableSigstoreRegistries,omitempty"`
 	AdditionalImages                   *[]Image       `json:"additionalImages,omitempty"`
 	BlockedImages                      *[]Image       `json:"blockedImages,omitempty"`
 	Operators                          *[]Operator    `json:"operators,omitempty"`