UPSTREAM: <carry>: [Default Catalog Tests]: Final cleanups and enhancements of initial implementation

Author: camilamacedo86

openshift/default-catalog-consistency/Makefile

@@ -1,5 +1,54 @@
+# Get the directory where this Makefile is, so we can use it below for including
+# Include the same Bingo variables used by the project
+DIR := $(strip $(shell dirname $(realpath $(lastword $(MAKEFILE_LIST)))))
+include $(DIR)/../../.bingo/Variables.mk
+
+#SECTION General
+
+# The help target prints out all targets with their descriptions organized
+# beneath their categories. The categories are represented by '#SECTION' and the
+# target descriptions by '#HELP' or '#EXHELP'. The awk commands is responsible for reading the
+# entire set of makefiles included in this invocation, looking for lines of the
+# file as xyz: #HELP something, and then pretty-format the target and help. Then,
+# if there's a line with #SECTION something, that gets pretty-printed as a category.
+# More info on the usage of ANSI control characters for terminal formatting:
+# https://en.wikipedia.org/wiki/ANSI_escape_code#SGR_parameters
+# More info on the awk command:
+# http://linuxcommand.org/lc3_adv_awk.php
+# The extended-help target uses '#EXHELP' as the delineator.
+
+.PHONY: help
+help: #HELP Display essential help.
+	@awk 'BEGIN {FS = ":[^#]*#HELP"; printf "\nUsage:\n  make \033[36m<target>\033[0m\n\n"} /^[a-zA-Z_0-9-]+:.*#HELP / { printf "  \033[36m%-17s\033[0m %s\n", $$1, $$2 } ' $(MAKEFILE_LIST)
+
+#SECTION Tests
+
 .PHONY: test-catalog
-test-catalog: ## Run the set of tests to validate the quality of catalogs
-	WHAT=catalog \
+test-catalog: #HELP Run the set of tests to validate the quality of catalogs
 	E2E_GINKGO_OPTS="$(if $(ARTIFACT_DIR),--output-dir='$(ARTIFACT_DIR)') --junit-report junit_e2e.xml" \
 	go test -count=1 -v ./test/validate/...;
+
+#SECTION Development
+
+.PHONY: verify #HELP To verify the code
+verify: tidy fmt vet lint
+
+.PHONY: tidy #HELP Run go mod tidy.
+tidy:
+	go mod tidy
+
+.PHONY: fmt
+fmt: #HELP Run go fmt against code.
+	go fmt ./...
+
+.PHONY: vet
+vet: #HELP Run go vet against code.
+	go vet ./...
+
+.PHONY: lint
+lint: $(GOLANGCI_LINT) #HELP Run golangci linter.
+	$(GOLANGCI_LINT) run
+
+.PHONY: fix-lint
+fix-lint: $(GOLANGCI_LINT) #HELP Fix lint issues
+	$(GOLANGCI_LINT) run --fix

openshift/default-catalog-consistency/README.md

@@ -7,4 +7,13 @@ Those tests are used to check the consistency of the default catalogs in OpenShi
 
 ```bash
 make test-catalog
+```
+
+### Before Pushing Changes
+
+Before pushing changes to the repository, you should verify that the tests pass. 
+This can be done by running:
+
+```bash
+make verify
 ```

openshift/default-catalog-consistency/pkg/check/check.go

@@ -4,14 +4,16 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"github/operator-framework-operator-controller/openshift/default-catalog-consistency/pkg/extract"
 	"io/fs"
 	"os"
 	"path/filepath"
 
 	specsgov1 "github.com/opencontainers/image-spec/specs-go/v1"
-	"github.com/operator-framework/operator-registry/alpha/declcfg"
 	"oras.land/oras-go/v2"
+
+	"github.com/operator-framework/operator-registry/alpha/declcfg"
+
+	"github/operator-framework-operator-controller/openshift/default-catalog-consistency/pkg/extract"
 )
 
 // Checks is a collection of checks to be performed on the catalog image.

openshift/default-catalog-consistency/pkg/check/image.go

@@ -9,7 +9,6 @@ import (
 	"strings"
 
 	"github.com/containers/image/v5/manifest"
-
 	specsgov1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"oras.land/oras-go/v2"
 )
@@ -57,7 +56,7 @@ func ImageHasLabels(expectedLabels map[string]string) ImageCheck {
 		}
 	}
 
-	var pairs []string
+	pairs := make([]string, 0, len(expectedLabels))
 	for k, v := range expectedLabels {
 		pairs = append(pairs, fmt.Sprintf("%s=%s", k, v))
 	}

openshift/default-catalog-consistency/pkg/extract/extract.go

@@ -41,15 +41,15 @@ func (r *ExtractedImage) Cleanup() {
 }
 
 // UnpackImage pulls the image, extracts it to disk, and opens it as an OCI store.
-func UnpackImage(ctx context.Context, imageRef, name string, sysCtx *types.SystemContext) (res *ExtractedImage, err error) {
+func UnpackImage(ctx context.Context, imageRef, name string, sysCtx *types.SystemContext) (*ExtractedImage, error) {
 	tmpDir, err := os.MkdirTemp("", fmt.Sprintf("oci-%s-", name))
 	if err != nil {
 		return nil, fmt.Errorf("create temp dir: %w", err)
 	}
 
 	var digestTag string
 
-	res, err = func() (*ExtractedImage, error) {
+	extracted, err := func() (*ExtractedImage, error) {
 		srcRef, err := docker.ParseReference("//" + imageRef)
 		if err != nil {
 			return nil, fmt.Errorf("parse image ref: %w", err)
@@ -59,7 +59,12 @@ func UnpackImage(ctx context.Context, imageRef, name string, sysCtx *types.Syste
 		if err != nil {
 			return nil, fmt.Errorf("create policy context: %w", err)
 		}
-		defer policyCtx.Destroy()
+		// Ensure policy context is cleaned up properly
+		defer func() {
+			if err := policyCtx.Destroy(); err != nil {
+				fmt.Printf("unable to destroy policy context: %s", err)
+			}
+		}()
 
 		canonicalRef, err := resolveCanonicalRef(ctx, srcRef, sysCtx)
 		if err != nil {
@@ -115,11 +120,13 @@ func UnpackImage(ctx context.Context, imageRef, name string, sysCtx *types.Syste
 	}()
 
 	if err != nil {
-		os.RemoveAll(tmpDir)
+		if err := os.RemoveAll(tmpDir); err != nil {
+			fmt.Printf("failed to remove temp dir: %v\n", err)
+		}
 		return nil, err
 	}
 
-	return res, nil
+	return extracted, nil
 }
 
 // extractLayers extracts the filesystem layers from the OCI image layout under the given digest tag.
@@ -166,8 +173,9 @@ func extractLayers(ctx context.Context, layoutPath, fsPath, tag string) error {
 
 			_, err := archive.Apply(ctx, fsPath, decompress, archive.WithFilter(func(hdr *tar.Header) (bool, error) {
 				// Clean up extended headers and enforce safe permissions
+				// This configuration allow to extract the image layers
+				// without the need of root permissions in CI environments
 				hdr.PAXRecords = nil
-				hdr.Xattrs = nil
 				hdr.Uid = os.Getuid()
 				hdr.Gid = os.Getgid()
 				if hdr.FileInfo().IsDir() {
@@ -183,10 +191,9 @@ func extractLayers(ctx context.Context, layoutPath, fsPath, tag string) error {
 			return nil
 		}()
 		if err != nil {
-			return fmt.Errorf("decompress layer %d: %w", i, err)
+			return err
 		}
 	}
-
 	return nil
 }
 
@@ -229,7 +236,7 @@ func loadPolicyContext(sourceContext *types.SystemContext, imageRef string) (*si
 	// if we need to validate the image signature then we will need to
 	// change it.
 	if err != nil {
-		fmt.Println(fmt.Sprintf("no default policy found for (%s), using insecure policy", imageRef))
+		fmt.Printf("no default policy found for (%s), using insecure policy \n", imageRef)
 		insecurePolicy := []byte(`{
 			"default": [{"type": "insecureAcceptAnything"}]
 		}`)

openshift/default-catalog-consistency/test/utils/utils.go

@@ -5,9 +5,10 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
-	"sigs.k8s.io/yaml"
 	"strings"
 
+	"sigs.k8s.io/yaml"
+
 	apiv1 "github.com/operator-framework/operator-controller/api/v1"
 )
 

openshift/default-catalog-consistency/test/validate/suite_test.go

@@ -3,13 +3,14 @@ package validate
 import (
 	"context"
 	"fmt"
-	"github.com/containers/image/v5/types"
 	"os"
 	"testing"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
+	"github.com/containers/image/v5/types"
+
 	"github/operator-framework-operator-controller/openshift/default-catalog-consistency/pkg/check"
 	"github/operator-framework-operator-controller/openshift/default-catalog-consistency/pkg/extract"
 	"github/operator-framework-operator-controller/openshift/default-catalog-consistency/test/utils"