feat: move external cronjob to run timely cleanup to goroutine+configable for internval of cleanup (#934)

zdtsw · web-flow · commit 44613ee0ce4e · 2026-06-02T17:48:06.000Z
&lt;!--- Provide a general summary of your changes in the Title above --&gt;

## Description
&lt;!--- Describe your changes in detail --&gt;
- save resource, no spin up job every 15mins
- less image to pull down for disconnect
- should cleanup endpoint later if needed
- less security concern as one less networkpolicy and patch
-  make internval configable:
- either pass as flag `--cleanup-internval `to main or env variable
`CLEANUP_INTERNVAL_MINUTES` in
      deployment for customized value
    - if not set any or set to '0', use default 15mins
    - if set to a negative value, skip cleanup goroutine totally

## How Has This Been Tested?
&lt;!--- Please describe in detail how you tested your changes. --&gt;
&lt;!--- Include details of your testing environment, and the tests you ran
to --&gt;
&lt;!--- see how your change affects other areas of the code, etc. --&gt;

## Merge criteria:
&lt;!--- This PR will be merged by any repository approver when it meets
all the points in the checklist --&gt;
&lt;!--- Go over all the following points, and put an `x` in all the boxes
that apply. --&gt;

- [ ] The commits are squashed in a cohesive manner and have meaningful
messages.
- [ ] Testing instructions have been added in the PR body (for PRs
involving changes that are not immediately obvious).
- [ ] The developer has manually tested the changes and verified that
the changes work


&lt;!-- This is an auto-generated comment: release notes by coderabbit.ai
--&gt;
## Summary by CodeRabbit

* **Refactor**
* API key cleanup moved from external scheduled jobs into a built-in
background task within the service.
* Removed external cleanup scheduling and related network restrictions;
deployment wiring simplified and related image/parameter removed.
* Added internal TODOs and small runtime shutdown sequencing
improvements.

* **Chores**
* Cleaned up deployment overlays, patches, and params to remove
now-unused cleanup artifacts.
&lt;!-- end of auto-generated comment: release notes by coderabbit.ai --&gt;

---------

Signed-off-by: Wen Zhou &lt;wenzhou@redhat.com&gt;
diff --git a/.github/hack/cleanup-odh.sh b/.github/hack/cleanup-odh.sh
@@ -100,8 +100,7 @@ cleanup_maas_resources() {
     kubectl delete authpolicy maas-api-auth-policy -n "$ns" --ignore-not-found 2>/dev/null || true
     kubectl delete httproute maas-api-route -n "$ns" --ignore-not-found 2>/dev/null || true
     kubectl delete destinationrule maas-api-backend-tls -n "$ns" --ignore-not-found 2>/dev/null || true
-    kubectl delete networkpolicy maas-api-cleanup-restrict maas-authorino-allow -n "$ns" --ignore-not-found 2>/dev/null || true
-    kubectl delete cronjob maas-api-key-cleanup -n "$ns" --ignore-not-found 2>/dev/null || true
+    kubectl delete networkpolicy maas-authorino-allow -n "$ns" --ignore-not-found 2>/dev/null || true
     kubectl delete role maas-api-db-secret maas-controller-leader-election-role -n "$ns" --ignore-not-found 2>/dev/null || true
     kubectl delete rolebinding maas-api-db-secret maas-controller-leader-election-rolebinding -n "$ns" --ignore-not-found 2>/dev/null || true
     kubectl delete serviceaccount maas-api maas-controller -n "$ns" --ignore-not-found 2>/dev/null || true
diff --git a/deployment/base/maas-api/core/cronjob-cleanup.yaml b/deployment/base/maas-api/core/cronjob-cleanup.yaml
diff --git a/deployment/base/maas-api/core/deployment.yaml b/deployment/base/maas-api/core/deployment.yaml
@@ -31,6 +31,8 @@ spec:
           value: "models-as-a-service"
         - name: METRICS_PORT
           value: "9090"
+        - name: CLEANUP_INTERVAL_MINUTES
+          value: "15"
         resources:
           requests:
             memory: "64Mi"
diff --git a/deployment/base/maas-api/core/kustomization.yaml b/deployment/base/maas-api/core/kustomization.yaml
@@ -4,8 +4,6 @@ kind: Kustomization
 resources:
   - deployment.yaml
   - service.yaml
-  - cronjob-cleanup.yaml
-  - networkpolicy-cleanup.yaml
 
 images:
   - name: maas-api
diff --git a/deployment/base/maas-api/core/networkpolicy-cleanup.yaml b/deployment/base/maas-api/core/networkpolicy-cleanup.yaml
diff --git a/deployment/base/maas-api/overlays/tls/cronjob-cleanup-patch.yaml b/deployment/base/maas-api/overlays/tls/cronjob-cleanup-patch.yaml
diff --git a/deployment/base/maas-api/overlays/tls/kustomization.yaml b/deployment/base/maas-api/overlays/tls/kustomization.yaml
@@ -8,8 +8,6 @@ resources:
 patches:
   - path: deployment-patch.yaml
   - path: service-patch.yaml
-  - path: cronjob-cleanup-patch.yaml
-  - path: networkpolicy-cleanup-patch.yaml
   - target:
       group: gateway.networking.k8s.io
       version: v1
diff --git a/deployment/base/maas-api/overlays/tls/networkpolicy-cleanup-patch.yaml b/deployment/base/maas-api/overlays/tls/networkpolicy-cleanup-patch.yaml
diff --git a/deployment/base/maas-controller/manager/manager.yaml b/deployment/base/maas-controller/manager/manager.yaml
@@ -57,12 +57,6 @@ spec:
                 key: payload-processing-image
                 name: maas-parameters
                 optional: true
-          - name: RELATED_IMAGE_UBI_MINIMAL_IMAGE
-            valueFrom:
-              configMapKeyRef:
-                key: maas-api-key-cleanup-image
-                name: maas-parameters
-                optional: true
         image: maas-controller
         name: manager
         imagePullPolicy: Always
diff --git a/deployment/overlays/odh/params.env b/deployment/overlays/odh/params.env
@@ -1,7 +1,6 @@
 maas-api-image=quay.io/opendatahub/maas-api:odh-stable
 maas-controller-image=quay.io/opendatahub/maas-controller:odh-stable
 payload-processing-image=quay.io/opendatahub/odh-ai-gateway-payload-processing:odh-stable
-maas-api-key-cleanup-image=registry.redhat.io/ubi9/ubi-minimal:9.7
 payload-processing-replicas=1
 gateway-namespace=openshift-ingress
 gateway-name=maas-default-gateway
diff --git a/maas-api/cmd/main.go b/maas-api/cmd/main.go
@@ -10,6 +10,7 @@ import (
 	"net/url"
 	"os"
 	"os/signal"
+	"sync"
 	"syscall"
 	"time"
 
@@ -113,12 +114,39 @@ func serve() error {
 	if err != nil {
 		return fmt.Errorf("failed to initialize token store: %w", err)
 	}
+	var cleanupWg sync.WaitGroup
 	defer func() {
+		cancel()
+		cleanupWg.Wait()
 		if err := store.Close(); err != nil {
 			log.Error("Failed to close token store", "error", err)
 		}
 	}()
 
+	if cfg.CleanupIntervalMinutes > 0 {
+		interval := time.Duration(cfg.CleanupIntervalMinutes) * time.Minute
+		log.Info("Ephemeral key cleanup enabled", "interval", cfg.CleanupIntervalMinutes)
+		cleanupWg.Go(func() {
+			ticker := time.NewTicker(interval)
+			defer ticker.Stop()
+			for {
+				select {
+				case <-ticker.C:
+					count, err := store.DeleteExpiredEphemeral(ctx)
+					if err != nil {
+						log.Error("Failed to cleanup expired ephemeral keys", "error", err)
+					} else if count > 0 {
+						log.Info("Ephemeral key cleanup completed", "deletedCount", count)
+					}
+				case <-ctx.Done():
+					return
+				}
+			}
+		})
+	} else {
+		log.Info("Ephemeral key cleanup disabled")
+	}
+
 	if err = registerHandlers(ctx, log, router, cfg, cluster, store); err != nil {
 		return fmt.Errorf("failed to register handlers: %w", err)
 	}
@@ -221,7 +249,7 @@ func registerHandlers(ctx context.Context, log *logger.Logger, router *gin.Engin
 	// Internal routes (no auth required - called by Authorino / CronJob)
 	internalRoutes := router.Group("/internal/v1")
 	internalRoutes.POST("/api-keys/validate", apiKeyHandler.ValidateAPIKeyHandler)
-	internalRoutes.POST("/api-keys/cleanup", apiKeyHandler.CleanupExpiredEphemeralKeys)
+	internalRoutes.POST("/api-keys/cleanup", apiKeyHandler.CleanupExpiredEphemeralKeys) // TODO: consider remove endpoint if not public access
 	internalRoutes.POST("/subscriptions/select", subscriptionHandler.SelectSubscription)
 
 	return nil
diff --git a/maas-api/internal/api_keys/handler.go b/maas-api/internal/api_keys/handler.go
@@ -465,6 +465,8 @@ func (h *Handler) SearchAPIKeys(c *gin.Context) {
 	c.JSON(http.StatusOK, response)
 }
 
+
+// TODO: cleanup unless we wanna keep /cleanup endpoint
 // CleanupExpiredEphemeralKeys handles POST /internal/v1/api-keys/cleanup
 // Deletes expired ephemeral API keys. Called by CronJob.
 // Access is restricted at the network level via NetworkPolicy.
diff --git a/maas-api/internal/api_keys/service.go b/maas-api/internal/api_keys/service.go
@@ -295,6 +295,7 @@ func (s *Service) BulkRevokeAPIKeys(ctx context.Context, username string) (int,
 	return s.store.InvalidateAll(ctx, username)
 }
 
+// TODO: cleanup unless we wanna keep /cleanup endpoint
 // CleanupExpiredEphemeral deletes expired ephemeral keys from storage.
 // Called by the internal cleanup endpoint (CronJob).
 func (s *Service) CleanupExpiredEphemeral(ctx context.Context) (int64, error) {
diff --git a/maas-api/internal/config/config.go b/maas-api/internal/config/config.go
@@ -56,6 +56,10 @@ type Config struct {
 	// Bounds memory usage under high-cardinality user traffic. Default: 8192.
 	SARCacheMaxSize int
 
+	// CleanupIntervalMinutes controls how often the cleanup goroutine deletes expired API keys
+	// -1 disables cleanup logic, any positive value sets the interval in minutes. Default: 15 mins
+	CleanupIntervalMinutes int
+
 	MetricsPort int
 
 	// Deprecated flag (backward compatibility with pre-TLS version)
@@ -71,6 +75,7 @@ func Load() *Config {
 	accessCheckTimeoutSeconds, _ := env.GetInt("ACCESS_CHECK_TIMEOUT_SECONDS", 15)
 	sarCacheMaxSize, _ := env.GetInt("SAR_CACHE_MAX_SIZE", constant.DefaultSARCacheMaxSize)
 	metricsPort, _ := env.GetInt("METRICS_PORT", constant.DefaultMetricsPort)
+	cleanupIntervalMinutes, _ := env.GetInt("CLEANUP_INTERVAL_MINUTES", constant.DefaultCleanupIntervalMinutes)
 
 	c := &Config{
 		Name:                      env.GetString("INSTANCE_NAME", gatewayName),
@@ -86,6 +91,7 @@ func Load() *Config {
 		APIKeyMaxExpirationDays:   maxExpirationDays,
 		AccessCheckTimeoutSeconds: accessCheckTimeoutSeconds,
 		SARCacheMaxSize:           sarCacheMaxSize,
+		CleanupIntervalMinutes:    cleanupIntervalMinutes,
 		MetricsPort:               metricsPort,
 		// Deprecated env var (backward compatibility with pre-TLS version)
 		deprecatedHTTPPort: env.GetString("PORT", ""),
@@ -111,6 +117,8 @@ func (c *Config) bindFlags(fs *flag.FlagSet) {
 	// Deprecated flag (backward compatibility with pre-TLS version)
 	fs.StringVar(&c.deprecatedHTTPPort, "port", c.deprecatedHTTPPort, "DEPRECATED: use --address with --secure=false")
 
+	fs.IntVar(&c.CleanupIntervalMinutes, "cleanup-interval",
+		c.CleanupIntervalMinutes, "Cleanup API key interval (-1 to disable, default 15) in minutes")
 	fs.BoolVar(&c.DebugMode, "debug", c.DebugMode, "Enable debug mode")
 	// Note: DBConnectionURL is loaded from K8s secret 'maas-db-config', not from CLI flag
 }
@@ -163,6 +171,10 @@ func (c *Config) Validate() error {
 		return errors.New("ACCESS_CHECK_TIMEOUT_SECONDS must be at least 1")
 	}
 
+	if c.CleanupIntervalMinutes == 0 {
+		c.CleanupIntervalMinutes = constant.DefaultCleanupIntervalMinutes
+	}
+
 	if c.MetricsPort < 1 || c.MetricsPort > 65535 {
 		return errors.New("METRICS_PORT must be between 1 and 65535")
 	}
diff --git a/maas-api/internal/config/config_test.go b/maas-api/internal/config/config_test.go
@@ -87,9 +87,10 @@ func TestLoad_EnvironmentVariables(t *testing.T) {
 // and default address assignment.
 func TestValidate(t *testing.T) {
 	tests := []struct {
-		name        string
-		cfg         Config
-		expectError string
+		name                  string
+		cfg                   Config
+		expectError           string
+		expectCleanupInterval int
 	}{
 		{
 			name: "missing DBConnectionURL returns error",
@@ -195,6 +196,54 @@ func TestValidate(t *testing.T) {
 			},
 			expectError: "must be at least 1",
 		},
+		// cleanup API key tests
+		{
+			name: "CleanupIntervalMinutes negative value, disables cleanup",
+			cfg: Config{
+				DBConnectionURL:           "postgresql://localhost/test",
+				APIKeyMaxExpirationDays:   30,
+				AccessCheckTimeoutSeconds: 15,
+				CleanupIntervalMinutes:    -10,
+				MetricsPort:               9090,
+				MaaSSubscriptionNamespace: "models-as-a-service",
+			},
+			expectCleanupInterval: -10,
+		},
+		{
+			name: "CleanupIntervalMinutes 0, set to default 15mins",
+			cfg: Config{
+				DBConnectionURL:           "postgresql://localhost/test",
+				APIKeyMaxExpirationDays:   30,
+				AccessCheckTimeoutSeconds: 15,
+				CleanupIntervalMinutes:    0,
+				MetricsPort:               9090,
+				MaaSSubscriptionNamespace: "models-as-a-service",
+			},
+			expectCleanupInterval: 15,
+		},
+		{
+			name: "CleanupIntervalMinutes not set, set to default too",
+			cfg: Config{
+				DBConnectionURL:           "postgresql://localhost/test",
+				APIKeyMaxExpirationDays:   30,
+				AccessCheckTimeoutSeconds: 15,
+				MetricsPort:               9090,
+				MaaSSubscriptionNamespace: "models-as-a-service",
+			},
+			expectCleanupInterval: 15,
+		},
+		{
+			name: "CleanupIntervalMinutes positive value, use value",
+			cfg: Config{
+				DBConnectionURL:           "postgresql://localhost/test",
+				APIKeyMaxExpirationDays:   30,
+				AccessCheckTimeoutSeconds: 15,
+				CleanupIntervalMinutes:    20,
+				MetricsPort:               9090,
+				MaaSSubscriptionNamespace: "models-as-a-service",
+			},
+			expectCleanupInterval: 20,
+		},
 		{
 			name: "MetricsPort zero returns error",
 			cfg: Config{
@@ -265,6 +314,10 @@ func TestValidate(t *testing.T) {
 					t.Errorf("expected Address %q for insecure config, got %q", DefaultInsecureAddr, tt.cfg.Address)
 				}
 			}
+
+			if tt.expectCleanupInterval != 0 && tt.cfg.CleanupIntervalMinutes != tt.expectCleanupInterval {
+				t.Errorf("expected CleanupIntervalMinutes %d, got %d", tt.expectCleanupInterval, tt.cfg.CleanupIntervalMinutes)
+			}
 		})
 	}
 }
diff --git a/maas-api/internal/constant/const.go b/maas-api/internal/constant/const.go
@@ -23,6 +23,9 @@ const (
 	// DefaultSARCacheMaxSize is the maximum number of entries in the SAR admin-check cache.
 	DefaultSARCacheMaxSize = 8192
 
+	// DefaultCleanupIntervalMinutes is the default interval for the ephemeral key cleanup goroutine.
+	DefaultCleanupIntervalMinutes = 15
+
 	// LLMInferenceService annotation keys for model metadata.
 	AnnotationGenAIUseCase  = "opendatahub.io/genai-use-case"
 	AnnotationDescription   = "openshift.io/description"
