diff --git a/go.mod b/go.mod index 78fb68f26..4868f3432 100644 --- a/go.mod +++ b/go.mod @@ -86,6 +86,7 @@ require ( github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // indirect github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect github.com/hashicorp/go-uuid v1.0.3 // indirect + github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect github.com/huandu/xstrings v1.5.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/jackc/pgio v1.0.0 // indirect diff --git a/go.sum b/go.sum index 6a22902b4..32a540e7d 100644 --- a/go.sum +++ b/go.sum @@ -194,6 +194,8 @@ github.com/grpc-ecosystem/grpc-gateway/v2 v2.29.0/go.mod h1:Hyl3n6Twe1hvtd9XUXDe github.com/hashicorp/go-uuid v1.0.2/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-uuid v1.0.3 h1:2gKiV6YVmrJ1i2CKKa9obLvRieoRGviZFL26PcT/Co8= github.com/hashicorp/go-uuid v1.0.3/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= +github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= +github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= github.com/huandu/xstrings v1.5.0 h1:2ag3IFq9ZDANvthTwTiqSSZLjDc+BedvHPAp5tJy2TI= github.com/huandu/xstrings v1.5.0/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= diff --git a/pkg/service/cluster/artifact_cache.go b/pkg/service/cluster/artifact_cache.go new file mode 100644 index 000000000..64bb31821 --- /dev/null +++ b/pkg/service/cluster/artifact_cache.go @@ -0,0 +1,58 @@ +package cluster + +import ( + "fmt" + + lru "github.com/hashicorp/golang-lru/v2" + "github.com/stackrox/infra/pkg/service/metrics" +) + +const ( + // Default cache size: estimate 10KB per artifact × 100 clusters × 2 artifacts = ~2MB + // Setting to 1000 entries to allow for growth + defaultCacheSize = 1000 +) + +// artifactCache provides thread-safe LRU caching of immutable GCS artifact contents. +// Since workflow artifacts don't change once written, no TTL is needed. +type artifactCache struct { + cache *lru.Cache[string, []byte] +} + +// newArtifactCache creates a new artifact cache with the specified size. +func newArtifactCache(size int) (*artifactCache, error) { + cache, err := lru.New[string, []byte](size) + if err != nil { + return nil, fmt.Errorf("failed to create LRU cache: %w", err) + } + + return &artifactCache{ + cache: cache, + }, nil +} + +// Get retrieves cached artifact content if present. +// Returns the content and true if found, nil and false otherwise. +func (c *artifactCache) Get(bucket, key string) ([]byte, bool) { + cacheKey := makeCacheKey(bucket, key) + content, found := c.cache.Get(cacheKey) + if !found { + metrics.ArtifactCacheMissesCounter.Inc() + return nil, false + } + + metrics.ArtifactCacheHitsCounter.Inc() + return content, true +} + +// Set stores artifact content in the cache. +func (c *artifactCache) Set(bucket, key string, content []byte) { + cacheKey := makeCacheKey(bucket, key) + c.cache.Add(cacheKey, content) + metrics.ArtifactCacheSizeGauge.Set(float64(c.cache.Len())) +} + +// makeCacheKey creates a unique cache key from bucket and object key. +func makeCacheKey(bucket, key string) string { + return fmt.Sprintf("%s/%s", bucket, key) +} diff --git a/pkg/service/cluster/artifact_cache_test.go b/pkg/service/cluster/artifact_cache_test.go new file mode 100644 index 000000000..3dae7a26c --- /dev/null +++ b/pkg/service/cluster/artifact_cache_test.go @@ -0,0 +1,94 @@ +package cluster + +import ( + "testing" +) + +func TestArtifactCache_GetSet(t *testing.T) { + cache, err := newArtifactCache(10) + if err != nil { + t.Fatalf("failed to create cache: %v", err) + } + + bucket := "test-bucket" + key := "test-key" + content := []byte("test content") + + // Cache miss on first get + _, found := cache.Get(bucket, key) + if found { + t.Error("expected cache miss, got hit") + } + + // Set content + cache.Set(bucket, key, content) + + // Cache hit on second get + retrieved, found := cache.Get(bucket, key) + if !found { + t.Error("expected cache hit, got miss") + } + if string(retrieved) != string(content) { + t.Errorf("expected content %q, got %q", content, retrieved) + } +} + +func TestArtifactCache_MultipleEntries(t *testing.T) { + cache, err := newArtifactCache(10) + if err != nil { + t.Fatalf("failed to create cache: %v", err) + } + + // Add multiple entries + entries := map[string][]byte{ + "bucket1/key1": []byte("content1"), + "bucket1/key2": []byte("content2"), + "bucket2/key1": []byte("content3"), + } + + for key, content := range entries { + cache.Set("bucket", key, content) + } + + // Verify all entries are cached + for key, expectedContent := range entries { + retrieved, found := cache.Get("bucket", key) + if !found { + t.Errorf("expected cache hit for key %q", key) + } + if string(retrieved) != string(expectedContent) { + t.Errorf("for key %q, expected %q, got %q", key, expectedContent, retrieved) + } + } +} + +func TestArtifactCache_LRUEviction(t *testing.T) { + cache, err := newArtifactCache(2) // Small cache for testing eviction + if err != nil { + t.Fatalf("failed to create cache: %v", err) + } + + // Fill cache to capacity + cache.Set("bucket", "key1", []byte("content1")) + cache.Set("bucket", "key2", []byte("content2")) + + // Add one more entry, should evict oldest + cache.Set("bucket", "key3", []byte("content3")) + + // key1 should have been evicted + _, found := cache.Get("bucket", "key1") + if found { + t.Error("expected key1 to be evicted") + } + + // key2 and key3 should still be present + _, found = cache.Get("bucket", "key2") + if !found { + t.Error("expected key2 to be present") + } + + _, found = cache.Get("bucket", "key3") + if !found { + t.Error("expected key3 to be present") + } +} diff --git a/pkg/service/cluster/cluster.go b/pkg/service/cluster/cluster.go index bd73b6caa..11bd9436c 100644 --- a/pkg/service/cluster/cluster.go +++ b/pkg/service/cluster/cluster.go @@ -81,6 +81,7 @@ type clusterImpl struct { argoClientCtx context.Context workflowNamespace string bqClient bqutil.BigQueryClient + artifactCache *artifactCache } var ( @@ -115,6 +116,13 @@ func NewClusterService(registry *flavor.Registry, signer *signer.Signer, slackCl resumeExpiredClusterInterval = 5 * time.Second } + // Initialize artifact cache + // Artifacts are immutable once written, so pure LRU eviction is sufficient + cache, err := newArtifactCache(defaultCacheSize) + if err != nil { + return nil, fmt.Errorf("failed to create artifact cache: %w", err) + } + impl := &clusterImpl{ k8sWorkflowsClient: k8sWorkflowsClient, k8sPodsClient: k8sPodsClient, @@ -126,6 +134,7 @@ func NewClusterService(registry *flavor.Registry, signer *signer.Signer, slackCl argoClientCtx: ctx, workflowNamespace: workflowNamespace, bqClient: bqClient, + artifactCache: cache, } go impl.startSlackCheck() @@ -152,13 +161,6 @@ func (s *clusterImpl) Info(_ context.Context, clusterID *v1.ResourceByID) (*v1.C // List implements ClusterService.List. func (s *clusterImpl) List(ctx context.Context, request *v1.ClusterListRequest) (*v1.ClusterListResponse, error) { - workflowList, err := s.argoWorkflowsClient.ListWorkflows(s.argoClientCtx, &workflowpkg.WorkflowListRequest{ - Namespace: s.workflowNamespace, - }) - if err != nil { - return nil, err - } - // Obtain the email of the current principal. var email string if user, found := middleware.UserFromContext(ctx); found { @@ -167,10 +169,30 @@ func (s *clusterImpl) List(ctx context.Context, request *v1.ClusterListRequest) email = svcacct.Email } + // Build label selector for server-side filtering + selector, err := buildLabelSelector(request, email) + if err != nil { + return nil, err + } + + listOpts := &metav1.ListOptions{} + if selectorStr := selector.String(); selectorStr != "" { + listOpts.LabelSelector = selectorStr + } + + workflowList, err := s.argoWorkflowsClient.ListWorkflows(s.argoClientCtx, &workflowpkg.WorkflowListRequest{ + Namespace: s.workflowNamespace, + ListOptions: listOpts, + }) + if err != nil { + return nil, err + } + clusters := make([]*v1.Cluster, 0, len(workflowList.Items)) - // Loop over all of the workflows, and keep only the ones that match our - // request criteria. + // Loop over workflows and apply client-side filters for fields that can't be + // filtered server-side (time-based expiration, prefix matching, workflow status). + // Server-side filtering (via label selectors) handles: owner, flavor, deleted status. for _, workflow := range workflowList.Items { // This cluster is expired, and we did not request to include expired // clusters. @@ -178,22 +200,12 @@ func (s *clusterImpl) List(ctx context.Context, request *v1.ClusterListRequest) continue } - // TODO(perf): move this to a listOption for the WorkflowListRequest to do the selection on K8s side - // This cluster is not ours, and we did not request to include all clusters. - if !request.All && GetOwner(&workflow) != email { - continue - } - + // Filter by prefix (done client-side as label selectors don't support prefix matching) if request.Prefix != "" && !strings.HasPrefix(getClusterIDFromWorkflow(&workflow), request.Prefix) { continue } - // If a filter for allowed flavors is active and the cluster is not one of the allowed, skip. - if len(request.AllowedFlavors) > 0 && !isClusterOneOfAllowedFlavors(&workflow, request.AllowedFlavors) { - continue - } - - // If a status filter exists and the cluster is not one of the allowed, skip. + // Filter by status (done client-side as status is computed from workflow phase) if len(request.AllowedStatuses) > 0 && !isClusterOneOfAllowedStatuses(&workflow, request.AllowedStatuses) { continue } @@ -346,12 +358,17 @@ func (s *clusterImpl) create(req *v1.CreateClusterRequest, owner, eventID string // Use the user supplied name as the root of Argo workflow name and the Infra cluster Id. clusterID, ok := req.Parameters["name"] - if ok { - workflow.GenerateName = clusterID + "-" - } else { + if !ok { return nil, fmt.Errorf("parameter 'name' was not provided") } + // Validate that cluster ID meets Kubernetes label value requirements + if err := validateClusterID(clusterID); err != nil { + return nil, status.Errorf(codes.InvalidArgument, "invalid cluster ID: %v", err) + } + + workflow.GenerateName = clusterID + "-" + // Make sure there is no running argo workflow for infra cluster with the same ID existingWorkflow, _ := s.getMostRecentArgoWorkflowFromClusterID(clusterID) if existingWorkflow != nil { @@ -408,6 +425,8 @@ func (s *clusterImpl) create(req *v1.CreateClusterRequest, owner, eventID string workflow.SetLabels(map[string]string{ labelClusterID: clusterID, + labelOwner: emailToLabelValue(owner), + labelFlavor: flav.GetID(), }) log.Log(logging.INFO, "will create an infra cluster", @@ -515,6 +534,32 @@ func (s *clusterImpl) Access() map[string]middleware.Access { } } +// setDeletedLabel marks a workflow as deleted by adding the deleted label. +// This is applied after destruction completes (FINISHED) so status polling +// during DESTROYING continues to work. +func (s *clusterImpl) setDeletedLabel(ctx context.Context, workflowName string) error { + // JSON Patch path uses ~1 to escape / in label names + labelPath := "/metadata/labels/" + strings.ReplaceAll(labelDeleted, "/", "~1") + deletedPatch := []map[string]any{ + { + "op": "add", + "path": labelPath, + "value": "true", + }, + } + patchBytes, err := json.Marshal(deletedPatch) + if err != nil { + return fmt.Errorf("failed to marshal deleted label patch: %w", err) + } + + _, err = s.k8sWorkflowsClient.Patch(ctx, workflowName, types.JSONPatchType, patchBytes, metav1.PatchOptions{}) + if err != nil { + return fmt.Errorf("failed to patch workflow with deleted label: %w", err) + } + + return nil +} + func (s *clusterImpl) Delete(ctx context.Context, req *v1.ResourceByID) (*empty.Empty, error) { owner, err := middleware.GetOwnerFromContext(ctx) if err != nil { @@ -627,8 +672,12 @@ func (s *clusterImpl) RegisterServiceHandler(ctx context.Context, mux *runtime.S func (s *clusterImpl) getMostRecentArgoWorkflowFromClusterID(clusterID string) (*v1alpha1.Workflow, error) { listOpts := &metav1.ListOptions{} labelSelector := labels.NewSelector() - req, _ := labels.NewRequirement(labelClusterID, selection.Equals, []string{clusterID}) - labelSelector = labelSelector.Add(*req) + clusterIDRequirement, err := labels.NewRequirement(labelClusterID, selection.Equals, []string{clusterID}) + if err != nil { + log.Log(logging.ERROR, "failed to build cluster ID requirement", "cluster-id", clusterID, "error", err) + return nil, err + } + labelSelector = labelSelector.Add(*clusterIDRequirement) listOpts.LabelSelector = labelSelector.String() workflowList, err := s.argoWorkflowsClient.ListWorkflows(s.argoClientCtx, &workflowpkg.WorkflowListRequest{ @@ -658,8 +707,14 @@ func (s *clusterImpl) cleanupExpiredClusters() { for ; ; time.Sleep(resumeExpiredClusterInterval) { start := time.Now() + // Use label selector to filter out already-deleted workflows server-side + labelSelector := fmt.Sprintf("%s!=%s", labelDeleted, "true") + workflowList, err := s.argoWorkflowsClient.ListWorkflows(s.argoClientCtx, &workflowpkg.WorkflowListRequest{ Namespace: s.workflowNamespace, + ListOptions: &metav1.ListOptions{ + LabelSelector: labelSelector, + }, }) if err != nil { log.Log(logging.ERROR, "failed to list workflows", "error", err) @@ -667,7 +722,15 @@ func (s *clusterImpl) cleanupExpiredClusters() { } for _, workflow := range workflowList.Items { - if workflowStatus(workflow.Status) != v1.Status_READY { + status := workflowStatus(workflow.Status) + if status == v1.Status_FINISHED { + if err := s.setDeletedLabel(s.argoClientCtx, workflow.GetName()); err != nil { + log.Log(logging.ERROR, "error occurred setting deleted label", "workflow-name", workflow.GetName(), "error", err) + } + continue + } + + if status != v1.Status_READY { continue } @@ -676,6 +739,7 @@ func (s *clusterImpl) cleanupExpiredClusters() { } log.Log(logging.INFO, "resuming an argo workflow that has expired", "workflow-name", workflow.GetName()) + _, err = s.argoWorkflowsClient.ResumeWorkflow(s.argoClientCtx, &workflowpkg.WorkflowResumeRequest{ Name: workflow.GetName(), Namespace: s.workflowNamespace, @@ -691,8 +755,8 @@ func (s *clusterImpl) cleanupExpiredClusters() { } } + // Log the duration of the loop if above the warning threshold to be aware of performance issues. if time.Since(start) > loopDurationWarning { - // TODO: why are we logging this? log.Log(logging.WARN, fmt.Sprintf("expire loop took %s", time.Since(start).String())) } } @@ -747,8 +811,14 @@ func (s *clusterImpl) startSlackCheck() { for ; ; time.Sleep(slackCheckInterval) { start := time.Now() + // Use label selector to filter out deleted workflows server-side + labelSelector := fmt.Sprintf("%s!=%s", labelDeleted, "true") + workflowList, err := s.argoWorkflowsClient.ListWorkflows(s.argoClientCtx, &workflowpkg.WorkflowListRequest{ Namespace: s.workflowNamespace, + ListOptions: &metav1.ListOptions{ + LabelSelector: labelSelector, + }, }) if err != nil { log.Log(logging.ERROR, "failed to list workflows", "error", err) @@ -759,8 +829,8 @@ func (s *clusterImpl) startSlackCheck() { s.slackCheckWorkflow(workflow) } + // Log the duration of the loop if above the warning threshold to be aware of performance issues. if time.Since(start) > loopDurationWarning { - // TODO: why are we logging this? log.Log(logging.WARN, fmt.Sprintf("slack loop took %s", time.Since(start).String())) } } diff --git a/pkg/service/cluster/helpers.go b/pkg/service/cluster/helpers.go index 755252449..1c19c0fc2 100644 --- a/pkg/service/cluster/helpers.go +++ b/pkg/service/cluster/helpers.go @@ -3,6 +3,7 @@ package cluster import ( "errors" "fmt" + "regexp" "slices" "strings" "time" @@ -12,6 +13,8 @@ import ( "github.com/stackrox/infra/pkg/logging" "github.com/stackrox/infra/pkg/slack" "google.golang.org/protobuf/types/known/timestamppb" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" ) func getClusterIDFromWorkflow(workflow *v1alpha1.Workflow) string { @@ -55,11 +58,6 @@ func isNearingExpiry(workflow v1alpha1.Workflow) bool { return time.Now().Add(nearExpiry).After(workflowExpiryTime) } -func isClusterOneOfAllowedFlavors(workflow *v1alpha1.Workflow, allowedFlavors []string) bool { - flavor := GetFlavor(workflow) - return slices.Contains(allowedFlavors, flavor) -} - func isClusterOneOfAllowedStatuses(workflow *v1alpha1.Workflow, allowedStatuses []v1.Status) bool { status := workflowStatus(workflow.Status) return slices.Contains(allowedStatuses, status) @@ -75,13 +73,6 @@ type metaCluster struct { SlackDM bool } -type artifactData struct { - Name string - Description string - Tags map[string]struct{} - Data []byte -} - // metaClusterFromWorkflow() converts an Argo workflow into an infra cluster // with additional, non-cluster, metadata. func (s *clusterImpl) metaClusterFromWorkflow(workflow v1alpha1.Workflow) (*metaCluster, error) { @@ -129,10 +120,10 @@ func (s *clusterImpl) getClusterDetailsFromArtifacts(cluster *v1.Cluster, workfl if meta, found := flavorMetadata[artifact.Name]; found { // And only artifacts that are tagged with url or connect. - if _, foundURL := meta.Tags[artifactTagURL]; !foundURL { - if _, foundConnect := meta.Tags[artifactTagConnect]; !foundConnect { - continue - } + _, foundURL := meta.Tags[artifactTagURL] + _, foundConnect := meta.Tags[artifactTagConnect] + if !foundURL && !foundConnect { + continue } bucket, key := handleArtifactMigration(workflow, artifact) @@ -140,9 +131,16 @@ func (s *clusterImpl) getClusterDetailsFromArtifacts(cluster *v1.Cluster, workfl continue } - contents, err := s.signer.Contents(bucket, key) - if err != nil { - return nil, err + // Check cache first before making GCS API call + contents, found := s.artifactCache.Get(bucket, key) + if !found { + // Cache miss - fetch from GCS and cache the result + var err error + contents, err = s.signer.Contents(bucket, key) + if err != nil { + return nil, err + } + s.artifactCache.Set(bucket, key, contents) } if _, found := meta.Tags[artifactTagURL]; found { @@ -278,3 +276,82 @@ func workflowFailureDetails(workflowStatus v1alpha1.WorkflowStatus) error { } return errors.New("") } + +// emailToLabelValue converts an email address to a Kubernetes label-safe value. +// Kubernetes label values must match ([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] and be at most 63 characters. +func emailToLabelValue(email string) string { + // Replace characters that aren't valid in Kubernetes labels + result := strings.ReplaceAll(email, "@", ".at.") + result = strings.ReplaceAll(result, "+", ".plus.") + + // Ensure max length of 63 characters + if len(result) > 63 { + result = result[:63] + } + + // Ensure it starts and ends with alphanumeric (trim invalid leading/trailing chars) + result = strings.TrimRight(result, "._-") + result = strings.TrimLeft(result, "._-") + + return result +} + +// validateClusterID validates that a cluster ID meets Kubernetes label value requirements. +// Kubernetes label values must match ([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] and be at most 63 characters. +func validateClusterID(clusterID string) error { + if clusterID == "" { + return fmt.Errorf("cluster ID cannot be empty") + } + + if len(clusterID) > 63 { + return fmt.Errorf("cluster ID %q exceeds maximum length of 63 characters (got %d)", clusterID, len(clusterID)) + } + + // Kubernetes label value regex: must start and end with alphanumeric, middle can have .-_ + validLabelValue := regexp.MustCompile(`^[A-Za-z0-9]([A-Za-z0-9._-]*[A-Za-z0-9])?$`) + if !validLabelValue.MatchString(clusterID) { + return fmt.Errorf("cluster ID %q contains invalid characters: must start and end with alphanumeric, and contain only alphanumeric, dot, dash, or underscore", clusterID) + } + + return nil +} + +// buildLabelSelector constructs a Kubernetes label selector from a ClusterListRequest. +// This enables server-side filtering to reduce the amount of data transferred and processed. +func buildLabelSelector(req *v1.ClusterListRequest, email string) (labels.Selector, error) { + selector := labels.NewSelector() + + // Filter out deleted workflows unless --expired is specified + // (deleted workflows are a subset of expired workflows) + if !req.Expired { + requirement, err := labels.NewRequirement(labelDeleted, selection.NotEquals, []string{"true"}) + if err != nil { + return nil, err + } + selector = selector.Add(*requirement) + } + + // Filter by owner if not requesting all clusters + if !req.All { + if email == "" { + return nil, fmt.Errorf("no authenticated user found for non-all cluster list request") + } + labelSafeEmail := emailToLabelValue(email) + requirement, err := labels.NewRequirement(labelOwner, selection.Equals, []string{labelSafeEmail}) + if err != nil { + return nil, err + } + selector = selector.Add(*requirement) + } + + // Filter by allowed flavors if specified + if len(req.AllowedFlavors) > 0 { + requirement, err := labels.NewRequirement(labelFlavor, selection.In, req.AllowedFlavors) + if err != nil { + return nil, err + } + selector = selector.Add(*requirement) + } + + return selector, nil +} diff --git a/pkg/service/cluster/helpers_test.go b/pkg/service/cluster/helpers_test.go new file mode 100644 index 000000000..f0a329f18 --- /dev/null +++ b/pkg/service/cluster/helpers_test.go @@ -0,0 +1,310 @@ +package cluster + +import ( + "regexp" + "strings" + "testing" + + v1 "github.com/stackrox/infra/generated/api/v1" +) + +func TestEmailToLabelValue(t *testing.T) { + tests := []struct { + name string + email string + expected string + }{ + { + name: "simple email", + email: "user@example.com", + expected: "user.at.example.com", + }, + { + name: "email with plus", + email: "user+tag@example.com", + expected: "user.plus.tag.at.example.com", + }, + { + name: "email with dot", + email: "user.name@example.com", + expected: "user.name.at.example.com", + }, + { + name: "long email that exceeds 63 chars", + email: "very-long-email-address-that-exceeds-the-maximum-length@example.com", + expected: "very-long-email-address-that-exceeds-the-maximum-length.at.exam", + }, + { + name: "complex email", + email: "user.name+test@subdomain.example.com", + expected: "user.name.plus.test.at.subdomain.example.com", + }, + { + name: "email starting with plus", + email: "+tag@example.com", + expected: "plus.tag.at.example.com", + }, + { + name: "long email truncating on dash", + email: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-@example.com", + expected: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + }, + { + name: "long email truncating on dot", + email: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.@example.com", + expected: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + }, + } + + // Kubernetes label value regex + labelValueRegex := regexp.MustCompile(`^[A-Za-z0-9]([A-Za-z0-9._-]*[A-Za-z0-9])?$`) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := emailToLabelValue(tt.email) + if result != tt.expected { + t.Errorf("emailToLabelValue(%q) = %q, want %q", tt.email, result, tt.expected) + } + if len(result) > 63 { + t.Errorf("emailToLabelValue(%q) returned %q with length %d, exceeds 63 chars", tt.email, result, len(result)) + } + if result != "" && !labelValueRegex.MatchString(result) { + t.Errorf("emailToLabelValue(%q) returned %q, not a valid K8s label value", tt.email, result) + } + }) + } +} + +func TestBuildLabelSelector_FilterDeletedWorkflows(t *testing.T) { + tests := []struct { + name string + request *v1.ClusterListRequest + email string + expectedClauses []string // Expected clauses in the selector (order-independent) + expectError bool + }{ + { + name: "empty email with All=false - should error", + request: &v1.ClusterListRequest{ + All: false, + Expired: false, + }, + email: "", + expectError: true, + }, + { + name: "empty email with All=false and Expired=true - should error", + request: &v1.ClusterListRequest{ + All: false, + Expired: true, + }, + email: "", + expectError: true, + }, + { + name: "with owner filter", + request: &v1.ClusterListRequest{ + All: false, + Expired: false, + }, + email: "test@example.com", + expectedClauses: []string{ + "infra.stackrox.com/deleted!=true", + "infra.stackrox.com/owner=test.at.example.com", + }, + }, + { + name: "empty email with flavor filter - should error", + request: &v1.ClusterListRequest{ + All: false, + Expired: false, + AllowedFlavors: []string{"gke-default", "eks-default"}, + }, + email: "", + expectError: true, + }, + { + name: "with owner and flavor filters", + request: &v1.ClusterListRequest{ + All: false, + Expired: false, + AllowedFlavors: []string{"gke-default"}, + }, + email: "user@example.com", + expectedClauses: []string{ + "infra.stackrox.com/deleted!=true", + "infra.stackrox.com/owner=user.at.example.com", + "infra.stackrox.com/flavor in (gke-default)", + }, + }, + { + name: "all flag - exclude deleted unless expired", + request: &v1.ClusterListRequest{ + All: true, + Expired: false, + }, + email: "user@example.com", + expectedClauses: []string{"infra.stackrox.com/deleted!=true"}, + }, + { + name: "expired and all flags - include deleted", + request: &v1.ClusterListRequest{ + All: true, + Expired: true, + }, + email: "user@example.com", + expectedClauses: []string{}, // No filters + }, + { + name: "empty email with expired and flavor filter - should error", + request: &v1.ClusterListRequest{ + All: false, + Expired: true, + AllowedFlavors: []string{"gke-default"}, + }, + email: "", + expectError: true, + }, + { + name: "All=true with empty email - should succeed", + request: &v1.ClusterListRequest{ + All: true, + Expired: false, + }, + email: "", + expectedClauses: []string{"infra.stackrox.com/deleted!=true"}, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + selector, err := buildLabelSelector(tt.request, tt.email) + + if tt.expectError { + if err == nil { + t.Errorf("buildLabelSelector() expected error but got none") + } + return + } + + if err != nil { + t.Errorf("buildLabelSelector() returned unexpected error: %v", err) + return + } + + result := selector.String() + + // For empty expected clauses, result should be empty + if len(tt.expectedClauses) == 0 { + if result != "" { + t.Errorf("expected empty selector, got %q", result) + } + return + } + + // Check that all expected clauses are present (order-independent) + for _, expectedClause := range tt.expectedClauses { + if !strings.Contains(result, expectedClause) { + t.Errorf("expected selector to contain %q, got %q", expectedClause, result) + } + } + }) + } +} + +func TestValidateClusterID(t *testing.T) { + tests := []struct { + name string + clusterID string + expectError bool + }{ + { + name: "valid simple ID", + clusterID: "my-cluster", + expectError: false, + }, + { + name: "valid ID with dots and underscores", + clusterID: "cluster-1.test_env", + expectError: false, + }, + { + name: "valid single character", + clusterID: "a", + expectError: false, + }, + { + name: "valid two characters", + clusterID: "ab", + expectError: false, + }, + { + name: "empty cluster ID", + clusterID: "", + expectError: true, + }, + { + name: "cluster ID with spaces", + clusterID: "my cluster", + expectError: true, + }, + { + name: "cluster ID too long (64 chars)", + clusterID: "a234567890123456789012345678901234567890123456789012345678901234", + expectError: true, + }, + { + name: "cluster ID exactly 63 chars", + clusterID: "a23456789012345678901234567890123456789012345678901234567890123", + expectError: false, + }, + { + name: "cluster ID starting with dash", + clusterID: "-cluster", + expectError: true, + }, + { + name: "cluster ID ending with dash", + clusterID: "cluster-", + expectError: true, + }, + { + name: "cluster ID starting with dot", + clusterID: ".cluster", + expectError: true, + }, + { + name: "cluster ID ending with dot", + clusterID: "cluster.", + expectError: true, + }, + { + name: "cluster ID with special characters", + clusterID: "cluster@test", + expectError: true, + }, + { + name: "cluster ID with slash", + clusterID: "cluster/test", + expectError: true, + }, + { + name: "cluster ID starting with underscore", + clusterID: "_cluster", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateClusterID(tt.clusterID) + if tt.expectError && err == nil { + t.Errorf("validateClusterID(%q) expected error but got none", tt.clusterID) + } + if !tt.expectError && err != nil { + t.Errorf("validateClusterID(%q) expected no error but got: %v", tt.clusterID, err) + } + }) + } +} diff --git a/pkg/service/cluster/labels.go b/pkg/service/cluster/labels.go index b10afd400..6c6aced26 100644 --- a/pkg/service/cluster/labels.go +++ b/pkg/service/cluster/labels.go @@ -4,6 +4,17 @@ const ( // labelClusterId is the label key used to map an infra cluster to // an argo workflow. labelClusterID = "infra.stackrox.com/cluster-id" + + // labelOwner is the label key for the cluster owner email. + labelOwner = "infra.stackrox.com/owner" + + // labelFlavor is the label key for the cluster flavor ID. + labelFlavor = "infra.stackrox.com/flavor" + + // labelDeleted is the label key to mark workflows that have finished destroying. + // This label is set after a workflow reaches FINISHED so that list queries can + // filter it out server-side without hiding in-progress destroy status. + labelDeleted = "infra.stackrox.com/deleted" ) // Labeled represents a type that has labels. diff --git a/pkg/service/metrics/metrics.go b/pkg/service/metrics/metrics.go index 6044098a7..c2482f634 100644 --- a/pkg/service/metrics/metrics.go +++ b/pkg/service/metrics/metrics.go @@ -13,9 +13,39 @@ var ( }, []string{"flavor"}, ) + + // ArtifactCacheHitsCounter tracks successful cache lookups for GCS artifacts + ArtifactCacheHitsCounter = prometheus.NewCounter( + prometheus.CounterOpts{ + Namespace: "infra", + Name: "artifact_cache_hits_total", + Help: "Total number of artifact cache hits", + }, + ) + + // ArtifactCacheMissesCounter tracks cache misses requiring GCS API calls + ArtifactCacheMissesCounter = prometheus.NewCounter( + prometheus.CounterOpts{ + Namespace: "infra", + Name: "artifact_cache_misses_total", + Help: "Total number of artifact cache misses", + }, + ) + + // ArtifactCacheSizeGauge reports current number of entries in the artifact cache + ArtifactCacheSizeGauge = prometheus.NewGauge( + prometheus.GaugeOpts{ + Namespace: "infra", + Name: "artifact_cache_size", + Help: "Current number of entries in the artifact cache", + }, + ) ) func init() { // Metrics have to be registered to be exposed: prometheus.MustRegister(FlavorsUsedCounter) + prometheus.MustRegister(ArtifactCacheHitsCounter) + prometheus.MustRegister(ArtifactCacheMissesCounter) + prometheus.MustRegister(ArtifactCacheSizeGauge) }