diff --git a/openshift-knative-operator/pkg/common/manifests.go b/openshift-knative-operator/pkg/common/manifests.go new file mode 100644 index 0000000000..78dbacc3e9 --- /dev/null +++ b/openshift-knative-operator/pkg/common/manifests.go @@ -0,0 +1,54 @@ +package common + +import ( + "net/url" + + "knative.dev/operator/pkg/apis/operator/base" +) + +// manifestPathStatus is the minimal component-status view needed to sanitize status.manifests. +type manifestPathStatus interface { + GetManifests() []string + SetManifests(manifests []string) +} + +// SanitizeManifestSpec clears the unsupported spec.manifests and spec.additionalManifests fields, +// reporting whether anything was cleared. The operator installs only its own bundled manifests. +func SanitizeManifestSpec(spec *base.CommonSpec) bool { + cleared := false + if len(spec.Manifests) > 0 { + spec.Manifests = nil + cleared = true + } + if len(spec.AdditionalManifests) > 0 { + spec.AdditionalManifests = nil + cleared = true + } + return cleared +} + +// SanitizeManifestStatus keeps only local paths in status.manifests, dropping any URL entry, +// reporting whether anything was dropped. The operator records only local (koData) paths there itself. +func SanitizeManifestStatus(status manifestPathStatus) bool { + paths := status.GetManifests() + kept := make([]string, 0, len(paths)) + for _, p := range paths { + if isRemoteURL(p) { + continue + } + kept = append(kept, p) + } + if len(kept) == len(paths) { + return false + } + status.SetManifests(kept) + return true +} + +// isRemoteURL reports whether p is an http(s) URL rather than a local filesystem path. +// Local (koData) paths never carry an http/https scheme, so the scheme check alone is +// sufficient to distinguish them. +func isRemoteURL(p string) bool { + u, err := url.Parse(p) + return err == nil && (u.Scheme == "http" || u.Scheme == "https") +} diff --git a/openshift-knative-operator/pkg/common/manifests_test.go b/openshift-knative-operator/pkg/common/manifests_test.go new file mode 100644 index 0000000000..85af54862e --- /dev/null +++ b/openshift-knative-operator/pkg/common/manifests_test.go @@ -0,0 +1,161 @@ +package common + +import ( + "testing" + + "knative.dev/operator/pkg/apis/operator/base" + "knative.dev/operator/pkg/apis/operator/v1beta1" +) + +// TestSanitizeManifestSpecClearsUserManifests verifies the unsupported spec.manifests and +// spec.additionalManifests fields are cleared. The operator installs only its own bundled manifests. +func TestSanitizeManifestSpecClearsUserManifests(t *testing.T) { + t.Parallel() + spec := &base.CommonSpec{ + Manifests: []base.Manifest{ + {Url: "https://example.com/manifests.yaml"}, + }, + AdditionalManifests: []base.Manifest{ + {Url: "https://example.com/additional-manifests.yaml"}, + }, + } + + if got := SanitizeManifestSpec(spec); !got { + t.Errorf("SanitizeManifestSpec() = %v, want true", got) + } + if spec.Manifests != nil { + t.Errorf("spec.Manifests = %v, want nil", spec.Manifests) + } + if spec.AdditionalManifests != nil { + t.Errorf("spec.AdditionalManifests = %v, want nil", spec.AdditionalManifests) + } +} + +// TestSanitizeManifestSpecClearsOnlySetFields verifies each field is handled independently: +// only the field that was actually set is cleared and counted. +func TestSanitizeManifestSpecClearsOnlySetFields(t *testing.T) { + t.Parallel() + t.Run("only spec.manifests set", func(t *testing.T) { + t.Parallel() + spec := &base.CommonSpec{ + Manifests: []base.Manifest{ + {Url: "https://example.com/manifests.yaml"}, + }, + } + if got := SanitizeManifestSpec(spec); !got { + t.Errorf("SanitizeManifestSpec() = %v, want true", got) + } + if spec.Manifests != nil { + t.Errorf("spec.Manifests = %v, want nil", spec.Manifests) + } + }) + + t.Run("only spec.additionalManifests set", func(t *testing.T) { + t.Parallel() + spec := &base.CommonSpec{ + AdditionalManifests: []base.Manifest{ + {Url: "https://example.com/additional-manifests.yaml"}, + }, + } + if got := SanitizeManifestSpec(spec); !got { + t.Errorf("SanitizeManifestSpec() = %v, want true", got) + } + if spec.AdditionalManifests != nil { + t.Errorf("spec.AdditionalManifests = %v, want nil", spec.AdditionalManifests) + } + }) + + t.Run("neither set", func(t *testing.T) { + t.Parallel() + spec := &base.CommonSpec{} + if got := SanitizeManifestSpec(spec); got { + t.Errorf("SanitizeManifestSpec() = %v, want false", got) + } + }) +} + +// TestSanitizeManifestStatusStripsRemoteURLs verifies URL entries are dropped from status.manifests +// while local filesystem paths are kept. +func TestSanitizeManifestStatusStripsRemoteURLs(t *testing.T) { + t.Parallel() + status := &v1beta1.KnativeServingStatus{} + status.SetManifests([]string{ + "/var/run/ko/knative-serving", // local path -> kept + "https://example.com/manifests.yaml", // URL -> dropped + "http://127.0.0.1:1/manifests.yaml", // URL -> dropped + }) + + if got := SanitizeManifestStatus(status); !got { + t.Errorf("SanitizeManifestStatus() = %v, want true", got) + } + got := status.GetManifests() + if len(got) != 1 || got[0] != "/var/run/ko/knative-serving" { + t.Errorf("status.manifests = %v, want [/var/run/ko/knative-serving]", got) + } +} + +// TestSanitizeManifestStatusKeepsLocalPaths verifies that a status made up entirely of local +// paths is left untouched and reports that nothing was dropped. +func TestSanitizeManifestStatusKeepsLocalPaths(t *testing.T) { + t.Parallel() + local := []string{ + "/var/run/ko/knative-serving/1.19.0", + "/var/run/ko/knative-serving/ingress", + } + status := &v1beta1.KnativeServingStatus{} + status.SetManifests(local) + + if got := SanitizeManifestStatus(status); got { + t.Errorf("SanitizeManifestStatus() = %v, want false", got) + } + got := status.GetManifests() + if len(got) != len(local) || got[0] != local[0] || got[1] != local[1] { + t.Errorf("status.manifests = %v, want %v", got, local) + } +} + +// TestSanitizeManifestStatusDropsAllRemote verifies that a status made up entirely of URLs is +// emptied and reports that something was dropped. +func TestSanitizeManifestStatusDropsAllRemote(t *testing.T) { + t.Parallel() + status := &v1beta1.KnativeServingStatus{} + status.SetManifests([]string{ + "https://example.com/a.yaml", + "https://example.com/b.yaml", + }) + + if got := SanitizeManifestStatus(status); !got { + t.Errorf("SanitizeManifestStatus() = %v, want true", got) + } + if got := status.GetManifests(); len(got) != 0 { + t.Errorf("status.manifests = %v, want empty", got) + } +} + +// TestIsRemoteURL verifies that only http/https URLs are treated as remote, while local +// filesystem paths (existing or not) are treated as local. +func TestIsRemoteURL(t *testing.T) { + t.Parallel() + tests := []struct { + name string + path string + want bool + }{ + {"https URL", "https://example.com/manifests.yaml", true}, + {"http URL", "http://127.0.0.1:1/manifests.yaml", true}, + {"comma-joined URLs", "https://a.example/x.yaml,https://b.example/y.yaml", true}, + {"absolute local path", "/var/run/ko/knative-serving/1.19.0", false}, + {"nonexistent local path", "/no/such/path/on/disk-xyz", false}, + {"relative local path", "knative-serving/1.19.0", false}, + {"file scheme", "file:///tmp/manifests.yaml", false}, + {"empty string", "", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := isRemoteURL(tt.path); got != tt.want { + t.Errorf("isRemoteURL(%q) = %v, want %v", tt.path, got, tt.want) + } + }) + } +} diff --git a/openshift-knative-operator/pkg/eventing/extension.go b/openshift-knative-operator/pkg/eventing/extension.go index 59cc49bd1e..cc45bfc7c6 100644 --- a/openshift-knative-operator/pkg/eventing/extension.go +++ b/openshift-knative-operator/pkg/eventing/extension.go @@ -114,6 +114,15 @@ func (e *extension) Reconcile(ctx context.Context, comp base.KComponent) error { ke.Spec.Registry.Override = images ke.Spec.Registry.Default = images["default"] + // spec.manifests/spec.additionalManifests are unsupported; clear them, as we force Registry + // above. Keep only local paths in status.manifests. + if common.SanitizeManifestSpec(&ke.Spec.CommonSpec) { + logging.FromContext(ctx).Warnw("Ignoring unsupported spec.manifests/spec.additionalManifests") + } + if common.SanitizeManifestStatus(ke.GetStatus()) { + logging.FromContext(ctx).Warnw("Dropping non-local status.manifests entries") + } + // Ensure webhook has 1G of memory. common.EnsureContainerMemoryLimit(&ke.Spec.CommonSpec, "eventing-webhook", resource.MustParse("1024Mi")) diff --git a/openshift-knative-operator/pkg/serving/extension.go b/openshift-knative-operator/pkg/serving/extension.go index 529a7610a2..3169815368 100644 --- a/openshift-knative-operator/pkg/serving/extension.go +++ b/openshift-knative-operator/pkg/serving/extension.go @@ -27,6 +27,7 @@ import ( kubeclient "knative.dev/pkg/client/injection/kube/client" deploymentinformer "knative.dev/pkg/client/injection/kube/informers/apps/v1/deployment" "knative.dev/pkg/controller" + "knative.dev/pkg/logging" "knative.dev/pkg/ptr" "knative.dev/pkg/reconciler" @@ -140,6 +141,15 @@ func (e *extension) Reconcile(ctx context.Context, comp base.KComponent) error { ks.Spec.Registry.Default = images["default"] common.Configure(&ks.Spec.CommonSpec, "deployment", "queue-sidecar-image", images["queue-proxy"]) + // spec.manifests/spec.additionalManifests are unsupported; clear them, as we force Registry + // above. Keep only local paths in status.manifests. + if common.SanitizeManifestSpec(&ks.Spec.CommonSpec) { + logging.FromContext(ctx).Warnw("Ignoring unsupported spec.manifests/spec.additionalManifests") + } + if common.SanitizeManifestStatus(ks.GetStatus()) { + logging.FromContext(ctx).Warnw("Dropping non-local status.manifests entries") + } + // Default to 2 replicas. if ks.Spec.HighAvailability == nil { ks.Spec.HighAvailability = &base.HighAvailability{