feat: time-restricted AccessRequests (#221)

* add TTL to AccessRequest spec

* implement AccessRequest TTL logic

Author: Diaphteiros

api/clusters/v1alpha1/accessrequest_types.go

@@ -41,6 +41,13 @@ type AccessRequestSpec struct {
 	// Exactly one of Token or OIDC must be set.
 	// +optional
 	OIDC *OIDCConfig `json:"oidc,omitempty"`
+
+	// TTL is the desired time-to-live for the granted access.
+	// The AccessRequest will be automatically deleted after the TTL has expired.
+	// Note that this value refers to the creation time of the AccessRequest, not the time the access was granted.
+	// Leave nil for unlimited TTL.
+	// +optional
+	TTL *metav1.Duration `json:"ttl,omitempty"`
 }
 
 type TokenConfig struct {

api/clusters/v1alpha1/zz_generated.deepcopy.go

@@ -400,6 +400,13 @@ spec:
                       type: object
                     type: array
                 type: object
+              ttl:
+                description: |-
+                  TTL is the desired time-to-live for the granted access.
+                  The AccessRequest will be automatically deleted after the TTL has expired.
+                  Note that this value refers to the creation time of the AccessRequest, not the time the access was granted.
+                  Leave nil for unlimited TTL.
+                type: string
             type: object
             x-kubernetes-validations:
             - message: clusterRef may not be removed once set

api/crds/manifests/clusters.openmcp.cloud_accessrequests.yaml

@@ -3,10 +3,12 @@ package accessrequest
 import (
 	"context"
 	"fmt"
+	"time"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -60,6 +62,7 @@ func (r *AccessRequestReconciler) Reconcile(ctx context.Context, req reconcile.R
 		UpdateStatus(ctx, r.PlatformCluster.Client(), rr)
 }
 
+//nolint:gocyclo
 func (r *AccessRequestReconciler) reconcile(ctx context.Context, req reconcile.Request) ReconcileResult {
 	log := logging.FromContextOrPanic(ctx)
 	// get AccessRequest resource
@@ -89,10 +92,40 @@ func (r *AccessRequestReconciler) reconcile(ctx context.Context, req reconcile.R
 		}
 	}
 
+	// ignore AccessRequests that are in deletion
+	if !ar.GetDeletionTimestamp().IsZero() {
+		log.Info("Ignoring AccessRequest, because it is in deletion")
+		return ReconcileResult{}
+	}
+
 	rr := ReconcileResult{
 		Object: ar,
 	}
 
+	// check if TTL is set and has expired
+	if ar.Spec.TTL != nil {
+		isExpired, expirationTime := hasExpiredTTL(ar)
+		if isExpired {
+			log.Info("AccessRequest TTL has expired, deleting AccessRequest", "expirationTime", expirationTime, "creationTime", ar.GetCreationTimestamp(), "ttl", ar.Spec.TTL.Duration)
+			if err := r.PlatformCluster.Client().Delete(ctx, ar); err != nil {
+				rr.ReconcileError = errutils.WithReason(fmt.Errorf("error deleting AccessRequest after TTL expiration: %w", err), cconst.ReasonPlatformClusterInteractionProblem)
+				return rr
+			}
+			// don't update the status after deletion, could become a race condition
+			rr.Object = nil
+			return rr
+		}
+		log.Debug("AccessRequest TTL not yet expired", "expirationTime", expirationTime)
+		// compute requeue after duration, which is the remaining time until expiration plus one second
+		rr.Result.RequeueAfter = time.Until(expirationTime) + time.Second
+	}
+
+	// check if this reconciliation was just a TTL check and no further action is required
+	if ctrlutils.HasLabel(ar, clustersv1alpha1.ProviderLabel) && ctrlutils.HasLabel(ar, clustersv1alpha1.ProfileLabel) && ar.Spec.ClusterRef != nil {
+		log.Info("AccessRequest already has provider and profile labels and a clusterRef, no further action required")
+		return rr
+	}
+
 	// get Cluster that the request refers to
 	c := &clustersv1alpha1.Cluster{}
 	if ar.Spec.ClusterRef != nil {
@@ -190,10 +223,14 @@ func (r *AccessRequestReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		For(&clustersv1alpha1.AccessRequest{}).
 		WithEventFilter(predicate.And(
 			// ignore resources that already have the provider AND profile label set
-			predicate.Not(predicate.And(
-				ctrlutils.HasLabelPredicate(clustersv1alpha1.ProviderLabel, ""),
-				ctrlutils.HasLabelPredicate(clustersv1alpha1.ProfileLabel, ""),
-			)),
+			// unless the TTL is set, in which case we need to check for expiration
+			predicate.Or(
+				predicate.Not(predicate.And(
+					ctrlutils.HasLabelPredicate(clustersv1alpha1.ProviderLabel, ""),
+					ctrlutils.HasLabelPredicate(clustersv1alpha1.ProfileLabel, ""),
+				)),
+				hasTTLPredicate(),
+			),
 			ctrlutils.LabelSelectorPredicate(r.Config.Selector.Completed()),
 			predicate.Or(
 				ctrlutils.GotAnnotationPredicate(apiconst.OperationAnnotation, apiconst.OperationAnnotationValueReconcile),
@@ -203,3 +240,44 @@ func (r *AccessRequestReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		)).
 		Complete(r)
 }
+
+// hasTTLPredicate returns true if the AccessRequest is not in deletion and has a TTL set.
+func hasTTLPredicate() predicate.Predicate {
+	hasTTL := func(obj client.Object) bool {
+		ar, ok := obj.(*clustersv1alpha1.AccessRequest)
+		if !ok {
+			return false
+		}
+		if !ar.DeletionTimestamp.IsZero() {
+			return false // no need to reconcile already deleted objects here
+		}
+		return ar.Spec.TTL != nil
+	}
+	return predicate.Funcs{
+		UpdateFunc: func(tue event.TypedUpdateEvent[client.Object]) bool {
+			return hasTTL(tue.ObjectNew)
+		},
+		CreateFunc: func(tce event.TypedCreateEvent[client.Object]) bool {
+			return hasTTL(tce.Object)
+		},
+		DeleteFunc: func(tde event.TypedDeleteEvent[client.Object]) bool {
+			return false
+		},
+		GenericFunc: func(tge event.TypedGenericEvent[client.Object]) bool {
+			return hasTTL(tge.Object)
+		},
+	}
+}
+
+// hasExpiredTTL returns true if the AccessRequest has a TTL set and the TTL has expired.
+// If the AccessRequest has a TTL set, the second return value is the expiration time, otherwise it is the zero time.
+func hasExpiredTTL(ar *clustersv1alpha1.AccessRequest) (bool, time.Time) {
+	if ar == nil {
+		return false, time.Time{}
+	}
+	if ar.Spec.TTL == nil {
+		return false, time.Time{}
+	}
+	expirationTime := ar.GetCreationTimestamp().Add(ar.Spec.TTL.Duration)
+	return time.Now().After(expirationTime), expirationTime
+}

internal/controllers/accessrequest/controller.go

@@ -1,9 +1,13 @@
 package accessrequest_test
 
 import (
+	"time"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -145,4 +149,53 @@ var _ = Describe("AccessRequest Controller", func() {
 		Expect(ar.Spec.ClusterRef).To(BeNil())
 	})
 
+	Context("TTL", func() {
+
+		It("should not delete an AccessRequest without TTL", func() {
+			env := testutils.NewEnvironmentBuilder().WithFakeClient(scheme).WithInitObjectPath("testdata", "test-06").WithReconcilerConstructor(arReconciler).Build()
+			ar := &clustersv1alpha1.AccessRequest{}
+			Expect(env.Client().Get(env.Ctx, ctrlutils.ObjectKey("mc-access-no-ttl", "bar"), ar)).To(Succeed())
+			arCopy := ar.DeepCopy()
+			env.ShouldReconcile(testutils.RequestFromObject(ar))
+			Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(ar), ar)).To(Succeed())
+			Expect(ar.Labels).To(Equal(arCopy.Labels))
+			Expect(ar.Annotations).To(Equal(arCopy.Annotations))
+			Expect(ar.DeletionTimestamp.IsZero()).To(BeTrue())
+			Expect(ar.Spec).To(Equal(arCopy.Spec))
+			Expect(ar.Status).To(Equal(arCopy.Status))
+		})
+
+		It("should delete an AccessRequest with expired TTL", func() {
+			env := testutils.NewEnvironmentBuilder().WithFakeClient(scheme).WithInitObjectPath("testdata", "test-06").WithReconcilerConstructor(arReconciler).Build()
+			ar := &clustersv1alpha1.AccessRequest{}
+			Expect(env.Client().Get(env.Ctx, ctrlutils.ObjectKey("mc-access-ttl", "bar"), ar)).To(Succeed())
+			env.ShouldReconcile(testutils.RequestFromObject(ar))
+			Eventually(func() error {
+				return env.Client().Get(env.Ctx, client.ObjectKeyFromObject(ar), ar)
+			}).Should(MatchError(apierrors.IsNotFound, "IsNotFound"))
+		})
+
+		It("should not delete an AccessRequest with non-expired TTL", func() {
+			env := testutils.NewEnvironmentBuilder().WithFakeClient(scheme).WithInitObjectPath("testdata", "test-06").WithReconcilerConstructor(arReconciler).Build()
+			ar := &clustersv1alpha1.AccessRequest{}
+			Expect(env.Client().Get(env.Ctx, ctrlutils.ObjectKey("mc-access-ttl", "bar"), ar)).To(Succeed())
+
+			// modify creation timestamp to be recent so that TTL is not expired yet
+			now := metav1.Now()
+			ar.SetCreationTimestamp(now)
+			Expect(env.Client().Update(env.Ctx, ar)).To(Succeed())
+
+			arCopy := ar.DeepCopy()
+			rr := env.ShouldReconcile(testutils.RequestFromObject(ar))
+			Expect(rr.RequeueAfter).To(BeNumerically("~", time.Hour+time.Second, time.Second))
+			Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(ar), ar)).To(Succeed())
+			Expect(ar.Labels).To(Equal(arCopy.Labels))
+			Expect(ar.Annotations).To(Equal(arCopy.Annotations))
+			Expect(ar.DeletionTimestamp.IsZero()).To(BeTrue())
+			Expect(ar.Spec).To(Equal(arCopy.Spec))
+			Expect(ar.Status).To(Equal(arCopy.Status))
+		})
+
+	})
+
 })

internal/controllers/accessrequest/controller_test.go

@@ -0,0 +1,23 @@
+apiVersion: clusters.openmcp.cloud/v1alpha1
+kind: AccessRequest
+metadata:
+  name: mc-access-no-ttl
+  namespace: bar
+  labels:
+    clusters.openmcp.cloud/profile: default
+    clusters.openmcp.cloud/provider: asdf
+  creationTimestamp: "2025-01-01T00:00:00Z"
+spec:
+  clusterRef:
+    name: my-cluster
+    namespace: foo
+  permissions:
+  - rules:
+    - apiGroups:
+      - "*"
+      resources:
+      - "*"
+      verbs:
+      - "*"
+status:
+  phase: Pending

internal/controllers/accessrequest/testdata/test-06/accessrequest-no-ttl.yaml

@@ -0,0 +1,24 @@
+apiVersion: clusters.openmcp.cloud/v1alpha1
+kind: AccessRequest
+metadata:
+  name: mc-access-ttl
+  namespace: bar
+  labels:
+    clusters.openmcp.cloud/profile: default
+    clusters.openmcp.cloud/provider: asdf
+  creationTimestamp: "2025-01-01T00:00:00Z"
+spec:
+  ttl: "1h"
+  clusterRef:
+    name: my-cluster
+    namespace: foo
+  permissions:
+  - rules:
+    - apiGroups:
+      - "*"
+      resources:
+      - "*"
+      verbs:
+      - "*"
+status:
+  phase: Pending

internal/controllers/accessrequest/testdata/test-06/accessrequest-ttl.yaml

@@ -0,0 +1,13 @@
+apiVersion: clusters.openmcp.cloud/v1alpha1
+kind: Cluster
+metadata:
+  name: my-cluster
+  namespace: foo
+spec:
+  profile: default
+  kubernetes:
+    version: "1.32"
+  purposes:
+  - test
+  tenancy: Exclusive
+  

internal/controllers/accessrequest/testdata/test-06/cluster.yaml

@@ -0,0 +1,18 @@
+apiVersion: clusters.openmcp.cloud/v1alpha1
+kind: ClusterProfile
+metadata:
+  generation: 1
+  name: default
+spec:
+  environment: default
+  providerConfigRef:
+    name: foobar
+  providerRef:
+    name: asdf
+  supportedVersions:
+  - version: 1.32.2
+  - version: 1.31.7
+  - deprecated: true
+    version: 1.31.6
+  - deprecated: true
+    version: 1.31.5