diff --git a/pkg/payload/payload.go b/pkg/payload/payload.go index 49104c4b4..ac931eb1f 100644 --- a/pkg/payload/payload.go +++ b/pkg/payload/payload.go @@ -179,19 +179,21 @@ func LoadUpdate(dir, releaseImage, excludeIdentifier string, includeTechPreview errs = append(errs, fmt.Errorf("parse %s: %w", file.Name(), err)) continue } + originalFilename := filepath.Base(file.Name()) + for i := range ms { + ms[i].OriginalFilename = originalFilename + } + // Filter out manifests that should be excluded based on annotation filteredMs := []manifest.Manifest{} for _, manifest := range ms { - if shouldExclude(excludeIdentifier, includeTechPreview, profile, &manifest) { + if err := include(excludeIdentifier, includeTechPreview, profile, &manifest); err != nil { + klog.V(5).Infof("excluding %s group=%s kind=%s namespace=%s name=%s: %v\n", manifest.OriginalFilename, manifest.GVK.Group, manifest.GVK.Kind, manifest.Obj.GetNamespace(), manifest.Obj.GetName(), err) continue } filteredMs = append(filteredMs, manifest) } - ms = filteredMs - for i := range ms { - ms[i].OriginalFilename = filepath.Base(file.Name()) - } - manifests = append(manifests, ms...) + manifests = append(manifests, filteredMs...) } } @@ -213,31 +215,34 @@ func LoadUpdate(dir, releaseImage, excludeIdentifier string, includeTechPreview return payload, nil } -func shouldExclude(excludeIdentifier string, includeTechPreview bool, profile string, manifest *manifest.Manifest) bool { +func include(excludeIdentifier string, includeTechPreview bool, profile string, manifest *manifest.Manifest) error { annotations := manifest.Obj.GetAnnotations() if annotations == nil { - return true + return errors.New("no annotations") } excludeAnnotation := fmt.Sprintf("exclude.release.openshift.io/%s", excludeIdentifier) - if annotations[excludeAnnotation] == "true" { - return true + if v := annotations[excludeAnnotation]; v == "true" { + return fmt.Errorf("%s=%s", excludeAnnotation, v) } - featureGateAnnotationValue, featureGateAnnotationExists := annotations["release.openshift.io/feature-gate"] + featureGateAnnotation := "release.openshift.io/feature-gate" + featureGateAnnotationValue, featureGateAnnotationExists := annotations[featureGateAnnotation] if featureGateAnnotationValue == "TechPreviewNoUpgrade" && !includeTechPreview { - return true + return fmt.Errorf("tech-preview excluded, and %s=%s", featureGateAnnotation, featureGateAnnotationValue) } // never include the manifest if the feature-gate annotation is outside of allowed values (only TechPreviewNoUpgrade is currently allowed) if featureGateAnnotationExists && featureGateAnnotationValue != "TechPreviewNoUpgrade" { - return true + return fmt.Errorf("unrecognized value %s=%s", featureGateAnnotation, featureGateAnnotationValue) } profileAnnotation := fmt.Sprintf("include.release.openshift.io/%s", profile) if val, ok := annotations[profileAnnotation]; ok && val == "true" { - return false + return nil + } else if ok { + return fmt.Errorf("unrecognized value %s=%s", profileAnnotation, val) } - return true + return fmt.Errorf("%s unset", profileAnnotation) } // ValidateDirectory checks if a directory can be a candidate update by diff --git a/pkg/payload/payload_test.go b/pkg/payload/payload_test.go index b723152d0..08ce1e479 100644 --- a/pkg/payload/payload_test.go +++ b/pkg/payload/payload_test.go @@ -1,6 +1,7 @@ package payload import ( + "errors" "io/ioutil" "path/filepath" "reflect" @@ -17,7 +18,7 @@ import ( "github.com/openshift/library-go/pkg/manifest" ) -func Test_loadUpdatePayload(t *testing.T) { +func TestLoadUpdate(t *testing.T) { type args struct { dir string releaseImage string @@ -132,7 +133,7 @@ func mustRead(path string) []byte { return data } -func Test_Exclude(t *testing.T) { +func Test_include(t *testing.T) { tests := []struct { name string exclude string @@ -140,7 +141,7 @@ func Test_Exclude(t *testing.T) { profile string annotations map[string]interface{} - isExcluded bool + expected error }{ { name: "exclusion identifier set", @@ -149,13 +150,13 @@ func Test_Exclude(t *testing.T) { annotations: map[string]interface{}{ "exclude.release.openshift.io/identifier": "true", "include.release.openshift.io/self-managed-high-availability": "true"}, - isExcluded: true, + expected: errors.New("exclude.release.openshift.io/identifier=true"), }, { name: "profile selection works", profile: "single-node", annotations: map[string]interface{}{"include.release.openshift.io/self-managed-high-availability": "true"}, - isExcluded: true, + expected: errors.New("include.release.openshift.io/single-node unset"), }, { name: "profile selection works included", @@ -169,7 +170,7 @@ func Test_Exclude(t *testing.T) { "include.release.openshift.io/self-managed-high-availability": "true", "release.openshift.io/feature-gate": "TechPreviewNoUpgrade", }, - isExcluded: true, + expected: errors.New("tech-preview excluded, and release.openshift.io/feature-gate=TechPreviewNoUpgrade"), }, { name: "correct techpreview value is included if techpreview on", @@ -179,7 +180,6 @@ func Test_Exclude(t *testing.T) { "include.release.openshift.io/self-managed-high-availability": "true", "release.openshift.io/feature-gate": "TechPreviewNoUpgrade", }, - isExcluded: false, }, { name: "incorrect techpreview value is not excluded if techpreview off", @@ -188,7 +188,7 @@ func Test_Exclude(t *testing.T) { "include.release.openshift.io/self-managed-high-availability": "true", "release.openshift.io/feature-gate": "Other", }, - isExcluded: true, + expected: errors.New("unrecognized value release.openshift.io/feature-gate=Other"), }, { name: "incorrect techpreview value is not excluded if techpreview on", @@ -198,19 +198,19 @@ func Test_Exclude(t *testing.T) { "include.release.openshift.io/self-managed-high-availability": "true", "release.openshift.io/feature-gate": "Other", }, - isExcluded: true, + expected: errors.New("unrecognized value release.openshift.io/feature-gate=Other"), }, { name: "default profile selection excludes without annotation", profile: DefaultClusterProfile, annotations: map[string]interface{}{}, - isExcluded: true, + expected: errors.New("include.release.openshift.io/self-managed-high-availability unset"), }, { name: "default profile selection excludes with no annotation", profile: DefaultClusterProfile, annotations: nil, - isExcluded: true, + expected: errors.New("no annotations"), }, } for _, tt := range tests { @@ -219,15 +219,15 @@ func Test_Exclude(t *testing.T) { if tt.annotations != nil { metadata["annotations"] = tt.annotations } - ret := shouldExclude(tt.exclude, tt.includeTechPreview, tt.profile, &manifest.Manifest{ + err := include(tt.exclude, tt.includeTechPreview, tt.profile, &manifest.Manifest{ Obj: &unstructured.Unstructured{ Object: map[string]interface{}{ "metadata": metadata, }, }, }) - if ret != tt.isExcluded { - t.Errorf("(exclude: %v, profile: %v, annotations: %v) %v != %v", tt.exclude, tt.profile, tt.annotations, tt.isExcluded, ret) + if !reflect.DeepEqual(err, tt.expected) { + t.Errorf("(exclude: %v, profile: %v, annotations: %v) expected %v, but got %v", tt.exclude, tt.profile, tt.annotations, tt.expected, err) } }) }