caehe on unpack; fix image verification policy

Signed-off-by: grokspawn <jordan@nimblewidget.com>

Author: grokspawn

internal/catalogd/features/features.go

@@ -10,10 +10,12 @@ import (
 
 const (
 	APIV1MetasHandler = featuregate.Feature("APIV1MetasHandler")
+	GraphQLCatalogQueries = featuregate.Feature("GraphQLCatalogQueries")
 )
 
 var catalogdFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
 	APIV1MetasHandler: {Default: false, PreRelease: featuregate.Alpha},
+	GraphQLCatalogQueries: {Default: false, PreRelease: featuregate.Alpha},
 }
 
 var CatalogdFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate()

internal/catalogd/graphql/graphql.go

@@ -262,13 +262,27 @@ func analyzeBundleProperties(obj map[string]interface{}, info *SchemaInfo) {
 	}
 }
 
-// appendUnique adds a value to slice if not already present
+// appendUnique adds a value to slice if not already present, using JSON string as key for uniqueness
 func appendUnique(slice []interface{}, value interface{}) []interface{} {
+	seen := make(map[string]struct{}, len(slice))
+
 	for _, existing := range slice {
-		if reflect.DeepEqual(existing, value) {
-			return slice
+		key, err := json.Marshal(existing)
+		if err != nil {
+			continue // skip values that can't be marshaled
 		}
+		seen[string(key)] = struct{}{}
+	}
+
+	valueKey, err := json.Marshal(value)
+	if err != nil {
+		return slice // skip value if it can't be marshaled
+	}
+
+	if _, exists := seen[string(valueKey)]; exists {
+		return slice
 	}
+
 	return append(slice, value)
 }
 

internal/catalogd/server/handlers.go

@@ -9,8 +9,10 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/klog/v2"
 
 	"github.com/operator-framework/operator-controller/internal/catalogd/service"
 )
@@ -21,10 +23,10 @@ const timeFormat = "Mon, 02 Jan 2006 15:04:05 GMT"
 
 // CatalogHandlers handles HTTP requests for catalog content
 type CatalogHandlers struct {
-	store         CatalogStore
-	graphqlSvc    service.GraphQLService
-	rootURL       *url.URL
-	enableMetas   bool
+	store       CatalogStore
+	graphqlSvc  service.GraphQLService
+	rootURL     *url.URL
+	enableMetas bool
 }
 
 // Index provides methods for looking up catalog content by schema/package/name
@@ -183,7 +185,7 @@ func httpError(w http.ResponseWriter, err error) {
 		code = http.StatusInternalServerError
 	}
 	// Log the actual error for debugging
-	fmt.Printf("HTTP Error %d: %v\n", code, err)
+	klog.ErrorS(err, "HTTP error", "code", code)
 	http.Error(w, fmt.Sprintf("%d %s", code, http.StatusText(code)), code)
 }
 
@@ -206,7 +208,7 @@ func allowedMethodsHandler(next http.Handler, allowedMethods ...string) http.Han
 	allowedMethodSet := sets.New[string](allowedMethods...)
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// Allow POST requests only for GraphQL endpoint
-		if r.URL.Path != "" && len(r.URL.Path) >= 7 && r.URL.Path[len(r.URL.Path)-7:] != "graphql" && r.Method == http.MethodPost {
+		if !strings.HasSuffix(r.URL.Path, "graphql") && r.Method == http.MethodPost {
 			http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed)
 			return
 		}

internal/catalogd/server/http_helpers.go

@@ -11,7 +11,7 @@ func checkPreconditions(w http.ResponseWriter, r *http.Request, modtime time.Tim
 	// Check If-Modified-Since
 	if r.Method == http.MethodGet || r.Method == http.MethodHead {
 		if t, err := time.Parse(http.TimeFormat, r.Header.Get("If-Modified-Since")); err == nil {
-			// The Date-Modified header truncates sub-second precision, so
+			// The Last-Modified header truncates sub-second precision, so
 			// use ModTime < t+1s instead of ModTime <= t to check for unmodified.
 			if modtime.Before(t.Add(time.Second)) {
 				w.WriteHeader(http.StatusNotModified)
@@ -23,7 +23,7 @@ func checkPreconditions(w http.ResponseWriter, r *http.Request, modtime time.Tim
 	// Check If-Unmodified-Since
 	if r.Method != http.MethodGet && r.Method != http.MethodHead {
 		if t, err := time.Parse(http.TimeFormat, r.Header.Get("If-Unmodified-Since")); err == nil {
-			// The Date-Modified header truncates sub-second precision, so
+			// The Last-Modified header truncates sub-second precision, so
 			// use ModTime >= t+1s instead of ModTime > t to check for modified.
 			if modtime.After(t.Add(-time.Second)) {
 				w.WriteHeader(http.StatusPreconditionFailed)

internal/catalogd/service/graphql_service.go

@@ -63,6 +63,7 @@ func (s *CachedGraphQLService) GetSchema(catalog string, catalogFS fs.FS) (*gql.
 // ExecuteQuery executes a GraphQL query against a catalog
 func (s *CachedGraphQLService) ExecuteQuery(catalog string, catalogFS fs.FS, query string) (*graphql.Result, error) {
 	// Get or build the schema
+	// TODO: prevent cache rebuild on this callpath
 	dynamicSchema, err := s.GetSchema(catalog, catalogFS)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get GraphQL schema: %w", err)
@@ -87,14 +88,18 @@ func (s *CachedGraphQLService) InvalidateCache(catalog string) {
 // buildSchemaFromFS builds a GraphQL schema from a catalog filesystem
 func buildSchemaFromFS(catalogFS fs.FS) (*gql.DynamicSchema, error) {
 	var metas []*declcfg.Meta
+	var metasMux sync.Mutex
 
 	// Collect all metas from the catalog filesystem
+	// WalkMetasFS walks the filesystem concurrently, so we need to protect the metas slice
 	err := declcfg.WalkMetasFS(context.Background(), catalogFS, func(path string, meta *declcfg.Meta, err error) error {
 		if err != nil {
 			return err
 		}
 		if meta != nil {
+			metasMux.Lock()
 			metas = append(metas, meta)
+			metasMux.Unlock()
 		}
 		return nil
 	})

internal/catalogd/storage/localdir.go

@@ -31,7 +31,7 @@ type LocalDirV1 struct {
 	EnableMetasHandler bool
 
 	m sync.RWMutex
-	// this singleflight Group is used in `getIndex()` to handle concurrent HTTP requests
+	// this singleflight Group is used in `GetIndex()` to handle concurrent HTTP requests
 	// optimally. With the use of this singleflight group, the index is loaded from disk
 	// once per concurrent group of HTTP requests being handled by the metas handler.
 	// The single flight instance gives us a way to load the index from disk exactly once
@@ -124,6 +124,11 @@ func (s *LocalDirV1) Store(ctx context.Context, catalog string, fsys fs.FS) erro
 	// Invalidate GraphQL schema cache for this catalog
 	s.graphqlSvc.InvalidateCache(catalog)
 
+	// Pre-warm the GraphQL schema cache
+	if _, err := s.graphqlSvc.GetSchema(catalog, fsys); err != nil {
+		return fmt.Errorf("failed to pre-build GraphQL schema for catalog %q, %v", catalog, err)
+	}
+
 	return nil
 }
 

internal/shared/util/image/pull_test.go

@@ -40,7 +40,16 @@ func TestContainersImagePuller_Pull(t *testing.T) {
 	defer shutdown()
 
 	myModTime := time.Date(1985, 10, 25, 7, 53, 0, 0, time.FixedZone("PDT", -8*60*60))
-	defaultContextFunc := func(context.Context) (*types.SystemContext, error) { return &types.SystemContext{}, nil }
+
+	// Create a default context with insecure policy for tests that don't use buildSourceContextFunc
+	configDir := t.TempDir()
+	policyPath := filepath.Join(configDir, "policy.json")
+	insecurePolicy := `{"default":[{"type":"insecureAcceptAnything"}]}`
+	require.NoError(t, os.WriteFile(policyPath, []byte(insecurePolicy), 0644))
+
+	defaultContextFunc := func(context.Context) (*types.SystemContext, error) {
+		return &types.SystemContext{SignaturePolicyPath: policyPath}, nil
+	}
 
 	testCases := []struct {
 		name        string
@@ -298,8 +307,15 @@ func buildSourceContextFunc(t *testing.T, ref reference.Named) func(context.Cont
 		require.NoError(t, enc.Encode(registriesConf))
 		require.NoError(t, f.Close())
 
+		// Create an insecure policy for testing to override any system-level policy
+		// that might reject unsigned images
+		policyPath := filepath.Join(configDir, "policy.json")
+		insecurePolicy := `{"default":[{"type":"insecureAcceptAnything"}]}`
+		require.NoError(t, os.WriteFile(policyPath, []byte(insecurePolicy), 0644))
+
 		return &types.SystemContext{
 			SystemRegistriesConfPath: registriesConfPath,
+			SignaturePolicyPath:      policyPath,
 		}, nil
 	}
 }