From 835b4b7e92254e007106c90464abb3308b0454d8 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Thu, 5 Mar 2026 09:18:37 -0500 Subject: [PATCH 1/3] All CVO manifests in payload should be included --- pkg/payload/render_test.go | 48 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/pkg/payload/render_test.go b/pkg/payload/render_test.go index 909d115472..33538c5ce6 100644 --- a/pkg/payload/render_test.go +++ b/pkg/payload/render_test.go @@ -1,7 +1,9 @@ package payload import ( + "fmt" "os" + "path/filepath" "strings" "testing" @@ -12,6 +14,7 @@ import ( "k8s.io/utils/ptr" configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/library-go/pkg/manifest" ) func TestRenderManifest(t *testing.T) { @@ -304,3 +307,48 @@ data: return yaml } +func Test_cvoManifests(t *testing.T) { + installDir := filepath.Join("../../install") + files, err := os.ReadDir(installDir) + if err != nil { + t.Fatalf("failed to read directory: %v", err) + } + + if len(files) == 0 { + t.Fatalf("no files found in %s", installDir) + } + + var manifestsWithoutIncludeAnnotation []manifest.Manifest + const prefix = "include.release.openshift.io/" + for _, manifestFile := range files { + if manifestFile.IsDir() { + continue + } + filePath := filepath.Join(installDir, manifestFile.Name()) + manifests, err := manifest.ManifestsFromFiles([]string{filePath}) + if err != nil { + t.Fatalf("failed to load manifests: %v", err) + } + + for _, m := range manifests { + var found bool + for k := range m.Obj.GetAnnotations() { + if strings.HasPrefix(k, prefix) { + found = true + break + } + } + if !found { + manifestsWithoutIncludeAnnotation = append(manifestsWithoutIncludeAnnotation, m) + } + } + } + + if len(manifestsWithoutIncludeAnnotation) > 0 { + var messages []string + for _, m := range manifestsWithoutIncludeAnnotation { + messages = append(messages, fmt.Sprintf("%s/%s/%s/%s", m.OriginalFilename, m.GVK, m.Obj.GetName(), m.Obj.GetNamespace())) + } + t.Fatalf("Those manifests have no annotation with prefix %q and will not beinstalled by CVO: %s", prefix, strings.Join(messages, "', '")) + } +} From 059f3f09ec01e3fef7e7bba67d9e45c2aa1aa396 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Thu, 5 Mar 2026 16:25:28 -0500 Subject: [PATCH 2/3] Check bootstrap CVO too Because `bootstrap-pod.yaml` is not a valid yaml [1], we have to render it and then parse out manifests from it. We could make it valid as we did for the others, but it might be dangerous to do so. We have only one manifest for bootstrap. I am fine with a rendering step. https://github.com/openshift/cluster-version-operator/pull/1171#issuecomment-2813241542 --- pkg/payload/render_test.go | 99 +++++++++++++++++++++++++------------- 1 file changed, 65 insertions(+), 34 deletions(-) diff --git a/pkg/payload/render_test.go b/pkg/payload/render_test.go index 33538c5ce6..c4e2af0416 100644 --- a/pkg/payload/render_test.go +++ b/pkg/payload/render_test.go @@ -1,6 +1,7 @@ package payload import ( + "bytes" "fmt" "os" "path/filepath" @@ -308,47 +309,77 @@ data: return yaml } func Test_cvoManifests(t *testing.T) { - installDir := filepath.Join("../../install") - files, err := os.ReadDir(installDir) - if err != nil { - t.Fatalf("failed to read directory: %v", err) + config := manifestRenderConfig{ + ReleaseImage: "quay.io/cvo/release:latest", + ClusterProfile: "some-profile", } - if len(files) == 0 { - t.Fatalf("no files found in %s", installDir) + tests := []struct { + name string + dir string + }{ + { + name: "install dir", + dir: filepath.Join("../../install"), + }, + { + name: "bootstrap dir", + dir: filepath.Join("../../bootstrap"), + }, } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + files, err := os.ReadDir(tt.dir) + if err != nil { + t.Fatalf("failed to read directory: %v", err) + } - var manifestsWithoutIncludeAnnotation []manifest.Manifest - const prefix = "include.release.openshift.io/" - for _, manifestFile := range files { - if manifestFile.IsDir() { - continue - } - filePath := filepath.Join(installDir, manifestFile.Name()) - manifests, err := manifest.ManifestsFromFiles([]string{filePath}) - if err != nil { - t.Fatalf("failed to load manifests: %v", err) - } + if len(files) == 0 { + t.Fatalf("no files found in %s", tt.dir) + } - for _, m := range manifests { - var found bool - for k := range m.Obj.GetAnnotations() { - if strings.HasPrefix(k, prefix) { - found = true - break + var manifestsWithoutIncludeAnnotation []manifest.Manifest + const prefix = "include.release.openshift.io/" + for _, manifestFile := range files { + if manifestFile.IsDir() { + continue + } + filePath := filepath.Join(tt.dir, manifestFile.Name()) + data, err := os.ReadFile(filePath) + if err != nil { + t.Fatalf("failed to read manifest file: %v", err) + } + data, err = renderManifest(config, data) + if err != nil { + t.Fatalf("failed to render manifest: %v", err) + } + manifests, err := manifest.ParseManifests(bytes.NewReader(data)) + if err != nil { + t.Fatalf("failed to load manifests: %v", err) + } + + for _, m := range manifests { + m.OriginalFilename = filePath + var found bool + for k := range m.Obj.GetAnnotations() { + if strings.HasPrefix(k, prefix) { + found = true + break + } + } + if !found { + manifestsWithoutIncludeAnnotation = append(manifestsWithoutIncludeAnnotation, m) + } } } - if !found { - manifestsWithoutIncludeAnnotation = append(manifestsWithoutIncludeAnnotation, m) - } - } - } - if len(manifestsWithoutIncludeAnnotation) > 0 { - var messages []string - for _, m := range manifestsWithoutIncludeAnnotation { - messages = append(messages, fmt.Sprintf("%s/%s/%s/%s", m.OriginalFilename, m.GVK, m.Obj.GetName(), m.Obj.GetNamespace())) - } - t.Fatalf("Those manifests have no annotation with prefix %q and will not beinstalled by CVO: %s", prefix, strings.Join(messages, "', '")) + if len(manifestsWithoutIncludeAnnotation) > 0 { + var messages []string + for _, m := range manifestsWithoutIncludeAnnotation { + messages = append(messages, fmt.Sprintf("%s/%s/%s/%s", m.OriginalFilename, m.GVK, m.Obj.GetName(), m.Obj.GetNamespace())) + } + t.Fatalf("Those manifests have no annotation with prefix %q and will not beinstalled by CVO: %s", prefix, strings.Join(messages, "', '")) + } + }) } } From f0e561d0006068154b1aba3a6940a157c47fb5b0 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Thu, 5 Mar 2026 16:27:52 -0500 Subject: [PATCH 3/3] Loop folder recusively --- pkg/payload/render_test.go | 49 ++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/pkg/payload/render_test.go b/pkg/payload/render_test.go index c4e2af0416..739ceda3cd 100644 --- a/pkg/payload/render_test.go +++ b/pkg/payload/render_test.go @@ -3,6 +3,7 @@ package payload import ( "bytes" "fmt" + "io/fs" "os" "path/filepath" "strings" @@ -327,24 +328,21 @@ func Test_cvoManifests(t *testing.T) { dir: filepath.Join("../../bootstrap"), }, } + const prefix = "include.release.openshift.io/" for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - files, err := os.ReadDir(tt.dir) - if err != nil { - t.Fatalf("failed to read directory: %v", err) - } - - if len(files) == 0 { - t.Fatalf("no files found in %s", tt.dir) - } + var count int + err := filepath.WalkDir(tt.dir, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } - var manifestsWithoutIncludeAnnotation []manifest.Manifest - const prefix = "include.release.openshift.io/" - for _, manifestFile := range files { - if manifestFile.IsDir() { - continue + if d.IsDir() { + return nil } - filePath := filepath.Join(tt.dir, manifestFile.Name()) + + var manifestsWithoutIncludeAnnotation []manifest.Manifest + filePath := filepath.Join(tt.dir, d.Name()) data, err := os.ReadFile(filePath) if err != nil { t.Fatalf("failed to read manifest file: %v", err) @@ -371,15 +369,26 @@ func Test_cvoManifests(t *testing.T) { manifestsWithoutIncludeAnnotation = append(manifestsWithoutIncludeAnnotation, m) } } - } - if len(manifestsWithoutIncludeAnnotation) > 0 { - var messages []string - for _, m := range manifestsWithoutIncludeAnnotation { - messages = append(messages, fmt.Sprintf("%s/%s/%s/%s", m.OriginalFilename, m.GVK, m.Obj.GetName(), m.Obj.GetNamespace())) + if len(manifestsWithoutIncludeAnnotation) > 0 { + var messages []string + for _, m := range manifestsWithoutIncludeAnnotation { + messages = append(messages, fmt.Sprintf("%s/%s/%s/%s", m.OriginalFilename, m.GVK, m.Obj.GetName(), m.Obj.GetNamespace())) + } + t.Errorf("Those manifests have no annotation with prefix %q and will not be installed by CVO: %s", prefix, strings.Join(messages, ", ")) } - t.Fatalf("Those manifests have no annotation with prefix %q and will not beinstalled by CVO: %s", prefix, strings.Join(messages, "', '")) + count++ + return nil + }) + + if err != nil { + t.Fatalf("failed to walk directory: %v", err) } + + if count == 0 { + t.Errorf("no files found in %s", tt.dir) + } + }) } }