diff --git a/approval/api/v1/approval_types.go b/approval/api/v1/approval_types.go index d1aacf373..76b26297d 100644 --- a/approval/api/v1/approval_types.go +++ b/approval/api/v1/approval_types.go @@ -37,7 +37,7 @@ type ApprovalSpec struct { Strategy ApprovalStrategy `json:"strategy"` // State defines the state of the approval - // +kubebuilder:validation:Enum=Pending;Semigranted;Granted;Rejected;Suspended + // +kubebuilder:validation:Enum=Pending;Semigranted;Granted;Rejected;Suspended;Expired // +kubebuilder:default=Pending State ApprovalState `json:"state"` @@ -57,7 +57,7 @@ type ApprovalStatus struct { AvailableTransitions AvailableTransitions `json:"availableTransitions,omitempty"` // LastState defines the last state of the approval - // +kubebuilder:validation:Enum=Pending;Semigranted;Granted;Rejected;Suspended + // +kubebuilder:validation:Enum=Pending;Semigranted;Granted;Rejected;Suspended;Expired // +kubebuilder:default=Pending LastState ApprovalState `json:"lastState,omitempty"` diff --git a/approval/api/v1/approvalexpiration_types.go b/approval/api/v1/approvalexpiration_types.go new file mode 100644 index 000000000..a5ed79244 --- /dev/null +++ b/approval/api/v1/approvalexpiration_types.go @@ -0,0 +1,78 @@ +// Copyright 2025 Deutsche Telekom IT GmbH +// +// SPDX-License-Identifier: Apache-2.0 + +package v1 + +import ( + "github.com/telekom/controlplane/common/pkg/reminder" + "github.com/telekom/controlplane/common/pkg/types" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// ApprovalExpirationSpec defines the desired state of ApprovalExpiration +type ApprovalExpirationSpec struct { + // Approval is a reference to the parent Approval resource + Approval types.ObjectRef `json:"approval"` + + // Expiration is the absolute date when the approval expires + Expiration metav1.Time `json:"expiration"` + + // Thresholds defines when reminders should be sent relative to the expiration deadline. + // For example: [{Before: "720h"}, {Before: "168h", Repeat: "24h"}] sends one reminder + // 30 days before expiration, then daily reminders starting 7 days before. + // +optional + Thresholds []reminder.Threshold `json:"thresholds,omitempty"` +} + +// ApprovalExpirationStatus defines the observed state of ApprovalExpiration +type ApprovalExpirationStatus struct { + // +listType=map + // +listMapKey=type + // +patchStrategy=merge + // +patchMergeKey=type + // +optional + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` + + // SentReminders tracks which reminder thresholds have been triggered and when + // +optional + SentReminders []reminder.SentReminder `json:"sentReminders,omitempty"` +} + +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status + +// ApprovalExpiration is the Schema for the approvalexpirations API +// +kubebuilder:printcolumn:name="Approval",type="string",JSONPath=".spec.approval.name",description="The parent Approval" +// +kubebuilder:printcolumn:name="Expiration",type="date",JSONPath=".spec.expiration",description="When the approval expires" +type ApprovalExpiration struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec ApprovalExpirationSpec `json:"spec,omitempty"` + Status ApprovalExpirationStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// ApprovalExpirationList contains a list of ApprovalExpiration +type ApprovalExpirationList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []ApprovalExpiration `json:"items"` +} + +// GetConditions returns the conditions of the ApprovalExpiration +func (ae *ApprovalExpiration) GetConditions() []metav1.Condition { + return ae.Status.Conditions +} + +// SetCondition sets the condition of the ApprovalExpiration +func (ae *ApprovalExpiration) SetCondition(condition metav1.Condition) bool { + return meta.SetStatusCondition(&ae.Status.Conditions, condition) +} + +func init() { + SchemeBuilder.Register(&ApprovalExpiration{}, &ApprovalExpirationList{}) +} diff --git a/approval/api/v1/builder/builder.go b/approval/api/v1/builder/builder.go index 27596a9b0..6fc3b4bdd 100644 --- a/approval/api/v1/builder/builder.go +++ b/approval/api/v1/builder/builder.go @@ -192,7 +192,7 @@ func (b *approvalBuilder) Build(ctx context.Context) (finalResult ApprovalResult approvalReq.Spec.State = v1.ApprovalStateGranted if len(approvalReq.Spec.Decisions) == 0 { approvalReq.Spec.Decisions = append(approvalReq.Spec.Decisions, v1.Decision{ - Name: "System", + Name: v1.SystemDecisionName, Comment: v1.AutoApprovedComment, ResultingState: v1.ApprovalStateGranted, }) @@ -241,11 +241,13 @@ func (b *approvalBuilder) Build(ctx context.Context) (finalResult ApprovalResult // Approval was found if approvalExists { log.V(2).Info("Approval exists") - isDenied := b.Approval.Spec.State == v1.ApprovalStateRejected || b.Approval.Spec.State == v1.ApprovalStateSuspended + isDenied := b.Approval.Spec.State == v1.ApprovalStateRejected || + b.Approval.Spec.State == v1.ApprovalStateSuspended || + b.Approval.Spec.State == v1.ApprovalStateExpired if isDenied { - log.V(1).Info("Approval is rejected or suspended and must not be provisioned") - b.Owner.SetCondition(newApprovalGrantedCondition(b.Approval.Spec.State, "Approval has been rejected or suspended")) + log.V(1).Info("Approval is rejected, suspended, or expired and must not be provisioned") + b.Owner.SetCondition(newApprovalGrantedCondition(b.Approval.Spec.State, "Approval has been rejected, suspended, or expired")) return ApprovalResultDenied, nil } } diff --git a/approval/api/v1/common_types.go b/approval/api/v1/common_types.go index 61e23c201..c375175c7 100644 --- a/approval/api/v1/common_types.go +++ b/approval/api/v1/common_types.go @@ -22,6 +22,9 @@ const ( ApprovalStrategyFourEyes ApprovalStrategy = "FourEyes" ) +// SystemDecisionName is the decision name used for system-generated decisions (auto-approval, expiration). +const SystemDecisionName = "System" + // AutoApprovedComment is the comment added to auto-approved ApprovalRequests. const AutoApprovedComment = "Auto-approved: The approval strategy does not require manual review." @@ -32,6 +35,7 @@ const ( ApprovalActionDeny ApprovalAction = "Deny" ApprovalActionSuspend ApprovalAction = "Suspend" ApprovalActionResume ApprovalAction = "Resume" + ApprovalActionExpire ApprovalAction = "Expire" ) func (a ApprovalAction) String() string { diff --git a/approval/api/v1/zz_generated.deepcopy.go b/approval/api/v1/zz_generated.deepcopy.go index 81991ad0e..44707158a 100644 --- a/approval/api/v1/zz_generated.deepcopy.go +++ b/approval/api/v1/zz_generated.deepcopy.go @@ -9,6 +9,7 @@ package v1 import ( + "github.com/telekom/controlplane/common/pkg/reminder" "github.com/telekom/controlplane/common/pkg/types" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -41,6 +42,118 @@ func (in *Approval) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ApprovalExpiration) DeepCopyInto(out *ApprovalExpiration) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ApprovalExpiration. +func (in *ApprovalExpiration) DeepCopy() *ApprovalExpiration { + if in == nil { + return nil + } + out := new(ApprovalExpiration) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ApprovalExpiration) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ApprovalExpirationList) DeepCopyInto(out *ApprovalExpirationList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ApprovalExpiration, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ApprovalExpirationList. +func (in *ApprovalExpirationList) DeepCopy() *ApprovalExpirationList { + if in == nil { + return nil + } + out := new(ApprovalExpirationList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ApprovalExpirationList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ApprovalExpirationSpec) DeepCopyInto(out *ApprovalExpirationSpec) { + *out = *in + in.Approval.DeepCopyInto(&out.Approval) + in.Expiration.DeepCopyInto(&out.Expiration) + if in.Thresholds != nil { + in, out := &in.Thresholds, &out.Thresholds + *out = make([]reminder.Threshold, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ApprovalExpirationSpec. +func (in *ApprovalExpirationSpec) DeepCopy() *ApprovalExpirationSpec { + if in == nil { + return nil + } + out := new(ApprovalExpirationSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ApprovalExpirationStatus) DeepCopyInto(out *ApprovalExpirationStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.SentReminders != nil { + in, out := &in.SentReminders, &out.SentReminders + *out = make([]reminder.SentReminder, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ApprovalExpirationStatus. +func (in *ApprovalExpirationStatus) DeepCopy() *ApprovalExpirationStatus { + if in == nil { + return nil + } + out := new(ApprovalExpirationStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ApprovalList) DeepCopyInto(out *ApprovalList) { *out = *in diff --git a/approval/cmd/main.go b/approval/cmd/main.go index b0809a05b..386c07171 100644 --- a/approval/cmd/main.go +++ b/approval/cmd/main.go @@ -22,6 +22,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" approvalv1 "github.com/telekom/controlplane/approval/api/v1" + "github.com/telekom/controlplane/approval/internal/config" "github.com/telekom/controlplane/approval/internal/controller" webhookv1 "github.com/telekom/controlplane/approval/internal/webhook/v1" notificationv1 "github.com/telekom/controlplane/notification/api/v1" @@ -79,6 +80,16 @@ func main() { ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + // Load expiration configuration + expirationConfig, err := config.LoadExpirationConfig() + if err != nil { + setupLog.Error(err, "unable to load expiration config") + os.Exit(1) + } + setupLog.Info("loaded expiration config", + "expirationDuration", expirationConfig.ExpirationDuration, + "thresholds", len(expirationConfig.DefaultThresholds)) + // if the enable-http2 flag is false (the default), http/2 should be disabled // due to its vulnerabilities. More specifically, disabling http/2 will // prevent from being vulnerable to the HTTP/2 Stream Cancellation and @@ -122,8 +133,9 @@ func main() { } if err = (&controller.ApprovalReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + ExpirationConfig: expirationConfig, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Approval") os.Exit(1) @@ -135,6 +147,13 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "ApprovalRequest") os.Exit(1) } + if err = (&controller.ApprovalExpirationReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "ApprovalExpiration") + os.Exit(1) + } if os.Getenv("ENABLE_WEBHOOKS") != "false" { if err = webhookv1.SetupApprovalRequestWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "ApprovalRequest") diff --git a/approval/config/crd/bases/approval.cp.ei.telekom.de_approvalexpirations.yaml b/approval/config/crd/bases/approval.cp.ei.telekom.de_approvalexpirations.yaml new file mode 100644 index 000000000..d80807210 --- /dev/null +++ b/approval/config/crd/bases/approval.cp.ei.telekom.de_approvalexpirations.yaml @@ -0,0 +1,213 @@ +# SPDX-FileCopyrightText: 2025 Deutsche Telekom IT GmbH +# +# SPDX-License-Identifier: Apache-2.0 +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.20.1 + name: approvalexpirations.approval.cp.ei.telekom.de +spec: + group: approval.cp.ei.telekom.de + names: + kind: ApprovalExpiration + listKind: ApprovalExpirationList + plural: approvalexpirations + singular: approvalexpiration + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: The parent Approval + jsonPath: .spec.approval.name + name: Approval + type: string + - description: When the approval expires + jsonPath: .spec.expiration + name: Expiration + type: date + name: v1 + schema: + openAPIV3Schema: + description: ApprovalExpiration is the Schema for the approvalexpirations + API + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: ApprovalExpirationSpec defines the desired state of ApprovalExpiration + properties: + approval: + description: Approval is a reference to the parent Approval resource + properties: + name: + type: string + namespace: + type: string + uid: + description: |- + UID is a type that holds unique ID values, including UUIDs. Because we + don't ONLY use UUIDs, this is an alias to string. Being a type captures + intent and helps make sure that UIDs and names do not get conflated. + type: string + required: + - name + - namespace + type: object + expiration: + description: Expiration is the absolute date when the approval expires + format: date-time + type: string + thresholds: + description: |- + Thresholds defines when reminders should be sent relative to the expiration deadline. + For example: [{Before: "720h"}, {Before: "168h", Repeat: "24h"}] sends one reminder + 30 days before expiration, then daily reminders starting 7 days before. + items: + description: Threshold defines when a reminder should fire relative + to a deadline. + properties: + before: + description: |- + Before is how long before the deadline this reminder fires. + For example, "720h" means 30 days before the deadline. + type: string + repeat: + description: |- + Repeat optionally re-sends the reminder at this interval once the + "Before" window is entered. For example, Before=168h + Repeat=24h + sends a reminder at 7d, 6d, 5d, … , 1d before the deadline. + If omitted, the reminder fires exactly once for this threshold. + type: string + required: + - before + type: object + type: array + required: + - approval + - expiration + type: object + status: + description: ApprovalExpirationStatus defines the observed state of ApprovalExpiration + properties: + conditions: + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + sentReminders: + description: SentReminders tracks which reminder thresholds have been + triggered and when + items: + description: SentReminder tracks a reminder that was sent for a + specific threshold. + properties: + ref: + description: Ref is the reference to the created notification + or other resource. + properties: + name: + type: string + namespace: + type: string + uid: + description: |- + UID is a type that holds unique ID values, including UUIDs. Because we + don't ONLY use UUIDs, this is an alias to string. Being a type captures + intent and helps make sure that UIDs and names do not get conflated. + type: string + required: + - name + - namespace + type: object + sentAt: + description: SentAt is when this reminder was last sent for + this threshold. + format: date-time + type: string + threshold: + description: |- + Threshold is the key identifying which threshold triggered this reminder. + It is the string representation of the Threshold.Before duration (e.g. "720h0m0s"). + type: string + required: + - ref + - sentAt + - threshold + type: object + type: array + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/approval/config/crd/bases/approval.cp.ei.telekom.de_approvals.yaml b/approval/config/crd/bases/approval.cp.ei.telekom.de_approvals.yaml index d87b8ac54..5db4cabe7 100644 --- a/approval/config/crd/bases/approval.cp.ei.telekom.de_approvals.yaml +++ b/approval/config/crd/bases/approval.cp.ei.telekom.de_approvals.yaml @@ -222,6 +222,7 @@ spec: - Granted - Rejected - Suspended + - Expired type: string strategy: default: Auto @@ -356,6 +357,7 @@ spec: - Granted - Rejected - Suspended + - Expired type: string notificationRefs: description: NotificationRefs is a reference to the notifications diff --git a/approval/config/crd/kustomization.yaml b/approval/config/crd/kustomization.yaml index 29bce0dd3..56dd3fa3d 100644 --- a/approval/config/crd/kustomization.yaml +++ b/approval/config/crd/kustomization.yaml @@ -8,6 +8,7 @@ resources: - bases/approval.cp.ei.telekom.de_approvals.yaml - bases/approval.cp.ei.telekom.de_approvalrequests.yaml +- bases/approval.cp.ei.telekom.de_approvalexpirations.yaml # +kubebuilder:scaffold:crdkustomizeresource patches: diff --git a/approval/config/rbac/role.yaml b/approval/config/rbac/role.yaml index c92f518d7..30f52f15d 100644 --- a/approval/config/rbac/role.yaml +++ b/approval/config/rbac/role.yaml @@ -9,6 +9,7 @@ metadata: rules: - apiGroups: - "" + - events.k8s.io resources: - events verbs: @@ -17,6 +18,7 @@ rules: - apiGroups: - approval.cp.ei.telekom.de resources: + - approvalexpirations - approvalrequests - approvals verbs: @@ -30,6 +32,7 @@ rules: - apiGroups: - approval.cp.ei.telekom.de resources: + - approvalexpirations/finalizers - approvalrequests/finalizers - approvals/finalizers verbs: @@ -37,6 +40,7 @@ rules: - apiGroups: - approval.cp.ei.telekom.de resources: + - approvalexpirations/status - approvalrequests/status - approvals/status verbs: diff --git a/approval/internal/condition/condition.go b/approval/internal/condition/condition.go index 6c7c28762..4940bfe3e 100644 --- a/approval/internal/condition/condition.go +++ b/approval/internal/condition/condition.go @@ -54,3 +54,12 @@ func NewSemigrantedCondition() metav1.Condition { Message: "Request has been partially approved, awaiting second approval", } } + +func NewExpiredCondition() metav1.Condition { + return metav1.Condition{ + Type: "Approved", + Status: metav1.ConditionFalse, + Reason: "Expired", + Message: "Request has expired and requires re-approval", + } +} diff --git a/approval/internal/condition/condition_test.go b/approval/internal/condition/condition_test.go new file mode 100644 index 000000000..ced427f9e --- /dev/null +++ b/approval/internal/condition/condition_test.go @@ -0,0 +1,115 @@ +// Copyright 2025 Deutsche Telekom IT GmbH +// +// SPDX-License-Identifier: Apache-2.0 + +package condition + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestConditions(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Condition Suite") +} + +var _ = Describe("Approval Conditions", func() { + // The "Approved" condition type should indicate whether the approval grants access + // Status=True means approved and access is granted + // Status=False means not approved or access is revoked + + Context("NewApprovedCondition", func() { + It("should return Approved=True for Granted state", func() { + cond := NewApprovedCondition() + Expect(cond.Type).To(Equal("Approved")) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal("Granted")) + Expect(cond.Message).To(ContainSubstring("granted")) + }) + }) + + Context("NewRejectedCondition", func() { + It("should return Approved=False for Rejected state", func() { + cond := NewRejectedCondition() + Expect(cond.Type).To(Equal("Approved")) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal("Rejected")) + Expect(cond.Message).To(ContainSubstring("rejected")) + }) + }) + + Context("NewPendingCondition", func() { + It("should return Approved=False for Pending state", func() { + cond := NewPendingCondition() + Expect(cond.Type).To(Equal("Approved")) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal("Pending")) + Expect(cond.Message).To(ContainSubstring("pending")) + }) + }) + + Context("NewSemigrantedCondition", func() { + It("should return Approved=False for Semigranted state", func() { + cond := NewSemigrantedCondition() + Expect(cond.Type).To(Equal("Approved")) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal("Semigranted")) + Expect(cond.Message).To(ContainSubstring("partially")) + }) + }) + + Context("NewSuspendedCondition", func() { + It("should return Approved=True for Suspended state", func() { + cond := NewSuspendedCondition() + Expect(cond.Type).To(Equal("Approved")) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal("Suspended")) + Expect(cond.Message).To(ContainSubstring("suspended")) + }) + }) + + Context("NewExpiredCondition", func() { + It("should return Approved=False for Expired state", func() { + cond := NewExpiredCondition() + Expect(cond.Type).To(Equal("Approved")) + Expect(cond.Status).To(Equal(metav1.ConditionFalse), "Expired approvals should not be considered approved") + Expect(cond.Reason).To(Equal("Expired")) + Expect(cond.Message).To(ContainSubstring("expired")) + }) + }) + + // Semantic validation: states that grant access vs states that don't + Describe("Semantic Validation", func() { + It("should have Approved=True only for states that grant access", func() { + // States that GRANT access + grantingStates := []metav1.Condition{ + NewApprovedCondition(), // Granted + NewSuspendedCondition(), // Suspended (debatable, but currently True) + } + + for _, cond := range grantingStates { + Expect(cond.Status).To(Equal(metav1.ConditionTrue), + "State %s should have Approved=True", cond.Reason) + } + }) + + It("should have Approved=False for states that deny access", func() { + // States that DENY or DON'T YET grant access + denyingStates := []metav1.Condition{ + NewPendingCondition(), // Not yet approved + NewSemigrantedCondition(), // Partially approved (not enough) + NewRejectedCondition(), // Explicitly denied + NewExpiredCondition(), // No longer approved + } + + for _, cond := range denyingStates { + Expect(cond.Status).To(Equal(metav1.ConditionFalse), + "State %s should have Approved=False", cond.Reason) + } + }) + }) +}) diff --git a/approval/internal/config/expiration.go b/approval/internal/config/expiration.go new file mode 100644 index 000000000..190796fd0 --- /dev/null +++ b/approval/internal/config/expiration.go @@ -0,0 +1,81 @@ +// Copyright 2025 Deutsche Telekom IT GmbH +// +// SPDX-License-Identifier: Apache-2.0 + +package config + +import ( + "time" + + "github.com/pkg/errors" + "github.com/spf13/viper" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/telekom/controlplane/common/pkg/reminder" +) + +// ExpirationConfig holds the configuration for approval expiration +type ExpirationConfig struct { + // ExpirationDuration is how long until an approval expires (from creation) + ExpirationDuration time.Duration + + // DefaultThresholds defines the reminder schedule for approvals + DefaultThresholds []reminder.Threshold +} + +// LoadExpirationConfig loads the expiration configuration from environment variables +func LoadExpirationConfig() (*ExpirationConfig, error) { + setExpirationConfigDefaults() + + viper.AutomaticEnv() + viper.SetEnvPrefix("APPROVAL") + + expirationMonths := viper.GetInt("EXPIRATION_PERIOD_MONTHS") + weeklyReminderMonths := viper.GetInt("EXPIRATION_WEEKLY_REMINDER_MONTHS") + dailyReminderWeeks := viper.GetInt("EXPIRATION_DAILY_REMINDER_WEEKS") + + if expirationMonths <= 0 { + return nil, errors.New("APPROVAL_EXPIRATION_PERIOD_MONTHS must be greater than 0") + } + if weeklyReminderMonths < 0 { + return nil, errors.New("APPROVAL_EXPIRATION_WEEKLY_REMINDER_MONTHS must be >= 0") + } + if dailyReminderWeeks < 0 { + return nil, errors.New("APPROVAL_EXPIRATION_DAILY_REMINDER_WEEKS must be >= 0") + } + + // Convert to durations + expirationDuration := time.Duration(expirationMonths) * 30 * 24 * time.Hour + + // Build default thresholds + var thresholds []reminder.Threshold + + // Weekly reminder threshold (if configured) + if weeklyReminderMonths > 0 { + weeklyBefore := time.Duration(weeklyReminderMonths) * 30 * 24 * time.Hour + thresholds = append(thresholds, reminder.Threshold{ + Before: metav1.Duration{Duration: weeklyBefore}, + }) + } + + // Daily reminder threshold (if configured) + if dailyReminderWeeks > 0 { + dailyBefore := time.Duration(dailyReminderWeeks) * 7 * 24 * time.Hour + dailyRepeat := metav1.Duration{Duration: 24 * time.Hour} + thresholds = append(thresholds, reminder.Threshold{ + Before: metav1.Duration{Duration: dailyBefore}, + Repeat: &dailyRepeat, + }) + } + + return &ExpirationConfig{ + ExpirationDuration: expirationDuration, + DefaultThresholds: thresholds, + }, nil +} + +func setExpirationConfigDefaults() { + viper.SetDefault("EXPIRATION_PERIOD_MONTHS", 12) + viper.SetDefault("EXPIRATION_WEEKLY_REMINDER_MONTHS", 1) + viper.SetDefault("EXPIRATION_DAILY_REMINDER_WEEKS", 1) +} diff --git a/approval/internal/controller/approval_controller.go b/approval/internal/controller/approval_controller.go index 36b306878..86a5d5a70 100644 --- a/approval/internal/controller/approval_controller.go +++ b/approval/internal/controller/approval_controller.go @@ -14,6 +14,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller" approvalv1 "github.com/telekom/controlplane/approval/api/v1" + "github.com/telekom/controlplane/approval/internal/config" approval_handler "github.com/telekom/controlplane/approval/internal/handler/approval" cconfig "github.com/telekom/controlplane/common/pkg/config" cc "github.com/telekom/controlplane/common/pkg/controller" @@ -22,8 +23,9 @@ import ( // ApprovalReconciler reconciles a Approval object type ApprovalReconciler struct { client.Client - Scheme *runtime.Scheme - Recorder record.EventRecorder + Scheme *runtime.Scheme + Recorder record.EventRecorder + ExpirationConfig *config.ExpirationConfig cc.Controller[*approvalv1.Approval] } @@ -42,7 +44,8 @@ func (r *ApprovalReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c // SetupWithManager sets up the controller with the Manager. func (r *ApprovalReconciler) SetupWithManager(mgr ctrl.Manager) error { r.Recorder = mgr.GetEventRecorderFor("approval-controller") - r.Controller = cc.NewController(&approval_handler.ApprovalHandler{}, r.Client, r.Recorder) + handler := approval_handler.NewHandler(r.Client, r.ExpirationConfig) + r.Controller = cc.NewController(handler, r.Client, r.Recorder) return ctrl.NewControllerManagedBy(mgr). For(&approvalv1.Approval{}). diff --git a/approval/internal/controller/approvalexpiration_controller.go b/approval/internal/controller/approvalexpiration_controller.go new file mode 100644 index 000000000..31e2af559 --- /dev/null +++ b/approval/internal/controller/approvalexpiration_controller.go @@ -0,0 +1,83 @@ +// Copyright 2025 Deutsche Telekom IT GmbH +// +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/events" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + + approvalv1 "github.com/telekom/controlplane/approval/api/v1" + "github.com/telekom/controlplane/approval/internal/handler/approvalexpiration" + cconfig "github.com/telekom/controlplane/common/pkg/config" + cc "github.com/telekom/controlplane/common/pkg/controller" +) + +// ApprovalExpirationReconciler reconciles an ApprovalExpiration object +type ApprovalExpirationReconciler struct { + client.Client + Scheme *runtime.Scheme + Recorder record.EventRecorder // Deprecated: kept for common controller compatibility + EventRecorder events.EventRecorder // New events API + + cc.Controller[*approvalv1.ApprovalExpiration] +} + +// +kubebuilder:rbac:groups=approval.cp.ei.telekom.de,resources=approvalexpirations,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=approval.cp.ei.telekom.de,resources=approvalexpirations/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=approval.cp.ei.telekom.de,resources=approvalexpirations/finalizers,verbs=update +// +kubebuilder:rbac:groups=approval.cp.ei.telekom.de,resources=approvals,verbs=get;list;watch;update;patch +// +kubebuilder:rbac:groups=notification.cp.ei.telekom.de,resources=notifications,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=notification.cp.ei.telekom.de,resources=notificationchannels,verbs=get;list;watch +// +kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patch +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch + +// Reconcile is part of the main kubernetes reconciliation loop +func (r *ApprovalExpirationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + return r.Controller.Reconcile(ctx, req, &approvalv1.ApprovalExpiration{}) +} + +// SetupWithManager sets up the controller with the Manager. +func (r *ApprovalExpirationReconciler) SetupWithManager(mgr ctrl.Manager) error { + // Use new events API (k8s.io/client-go/tools/events) + r.EventRecorder = mgr.GetEventRecorder("approvalexpiration-controller") + // TODO: Migrate common controller to use events.EventRecorder, then remove this adapter + r.Recorder = &eventsRecorderAdapter{events: r.EventRecorder} + + handler := approvalexpiration.NewHandler(r.Client) + r.Controller = cc.NewController(handler, r.Client, r.Recorder) + + return ctrl.NewControllerManagedBy(mgr). + For(&approvalv1.ApprovalExpiration{}). + WithOptions(controller.Options{ + MaxConcurrentReconciles: cconfig.MaxConcurrentReconciles, + RateLimiter: cc.NewRateLimiter(), + }). + Complete(r) +} + +// eventsRecorderAdapter adapts events.EventRecorder (new API) to record.EventRecorder (old API) +// for compatibility with common controller until it's migrated to the new API. +type eventsRecorderAdapter struct { + events events.EventRecorder +} + +func (a *eventsRecorderAdapter) Event(object runtime.Object, eventtype, reason, message string) { + a.events.Eventf(object, nil, eventtype, reason, "Event", message) +} + +func (a *eventsRecorderAdapter) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) { + a.events.Eventf(object, nil, eventtype, reason, "Event", messageFmt, args...) +} + +func (a *eventsRecorderAdapter) AnnotatedEventf(object runtime.Object, annotations map[string]string, eventtype, reason, messageFmt string, args ...interface{}) { + // events.EventRecorder doesn't support annotations in the same way, just forward to Eventf + a.events.Eventf(object, nil, eventtype, reason, "Event", messageFmt, args...) +} diff --git a/approval/internal/controller/approvalexpiration_controller_test.go b/approval/internal/controller/approvalexpiration_controller_test.go new file mode 100644 index 000000000..fd47ef204 --- /dev/null +++ b/approval/internal/controller/approvalexpiration_controller_test.go @@ -0,0 +1,247 @@ +// Copyright 2025 Deutsche Telekom IT GmbH +// +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "time" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + approvalv1 "github.com/telekom/controlplane/approval/api/v1" + "github.com/telekom/controlplane/common/pkg/condition" + commonconfig "github.com/telekom/controlplane/common/pkg/config" + ctypes "github.com/telekom/controlplane/common/pkg/types" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("ApprovalExpiration Controller", Ordered, func() { + const resourceName = "test-expiration-approval" + + typeNamespacedName := types.NamespacedName{ + Name: resourceName, + Namespace: testNamespace, + } + + decider := approvalv1.Decider{ + TeamName: "test--decider", + TeamEmail: "test@decider.com", + ApplicationRef: &ctypes.TypedObjectRef{ + TypeMeta: metav1.TypeMeta{}, + ObjectRef: ctypes.ObjectRef{ + Name: "decider-app-name", + Namespace: testNamespace, + UID: "", + }, + }, + } + + requester := approvalv1.Requester{ + TeamName: "test--requester", + TeamEmail: "max.mustermann@telekom.de", + Reason: "I need access to this API", + ApplicationRef: &ctypes.TypedObjectRef{ + TypeMeta: metav1.TypeMeta{}, + ObjectRef: ctypes.ObjectRef{ + Name: "requester-app-name", + Namespace: testNamespace, + UID: "", + }, + }, + } + + resource := ctypes.TypedObjectRef{ + TypeMeta: metav1.TypeMeta{ + Kind: "Subscription", + }, + ObjectRef: ctypes.ObjectRef{ + Name: resourceName, + Namespace: testNamespace, + }, + } + + BeforeAll(func() { + By("creating the Approval in GRANTED state") + approval := &approvalv1.Approval{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "approval.cp.ei.telekom.de/v1", + Kind: "Approval", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: resourceName, + Namespace: testNamespace, + Labels: map[string]string{ + commonconfig.EnvironmentLabelKey: testEnvironment, + }, + }, + Spec: approvalv1.ApprovalSpec{ + Strategy: approvalv1.ApprovalStrategySimple, + State: approvalv1.ApprovalStateGranted, + Requester: requester, + Target: resource, + Decider: decider, + Action: "subscribe", + Decisions: []approvalv1.Decision{ + { + Name: "Alice", + Email: "alice@telekom.de", + Comment: "Approved", + ResultingState: approvalv1.ApprovalStateGranted, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, approval)).To(Succeed()) + + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, typeNamespacedName, approval) + g.Expect(err).NotTo(HaveOccurred()) + readyCondition := meta.FindStatusCondition(approval.Status.Conditions, condition.ConditionTypeReady) + g.Expect(readyCondition).ToNot(BeNil()) + g.Expect(readyCondition.Status).To(Equal(metav1.ConditionTrue)) + }, timeout, interval).Should(Succeed()) + }) + + AfterAll(func() { + By("cleaning up test resources") + approval := &approvalv1.Approval{} + err := k8sClient.Get(ctx, typeNamespacedName, approval) + if err == nil { + Expect(k8sClient.Delete(ctx, approval)).To(Succeed()) + } + + expirationName := types.NamespacedName{ + Name: resourceName + "--expiration", + Namespace: testNamespace, + } + ae := &approvalv1.ApprovalExpiration{} + err = k8sClient.Get(ctx, expirationName, ae) + if err == nil { + Expect(k8sClient.Delete(ctx, ae)).To(Succeed()) + } + }) + + It("should create ApprovalExpiration when Approval is GRANTED", func() { + expirationName := types.NamespacedName{ + Name: resourceName + "--expiration", + Namespace: testNamespace, + } + + Eventually(func(g Gomega) { + ae := &approvalv1.ApprovalExpiration{} + err := k8sClient.Get(ctx, expirationName, ae) + g.Expect(err).NotTo(HaveOccurred()) + + By("checking ApprovalExpiration has correct owner reference") + g.Expect(ae.OwnerReferences).To(HaveLen(1)) + g.Expect(ae.OwnerReferences[0].Name).To(Equal(resourceName)) + g.Expect(ae.OwnerReferences[0].Kind).To(Equal("Approval")) + g.Expect(*ae.OwnerReferences[0].Controller).To(BeTrue()) + + By("checking ApprovalExpiration has expiration and thresholds set") + g.Expect(ae.Spec.Expiration.Time).To(BeTemporally(">", time.Now())) + g.Expect(ae.Spec.Thresholds).To(HaveLen(2)) + g.Expect(ae.Spec.Thresholds[0].Before.Duration).To(BeNumerically(">", 0)) + g.Expect(ae.Spec.Thresholds[1].Before.Duration).To(BeNumerically(">", 0)) + g.Expect(ae.Spec.Thresholds[1].Repeat).NotTo(BeNil()) + + By("checking ApprovalExpiration references the Approval") + g.Expect(ae.Spec.Approval.Name).To(Equal(resourceName)) + g.Expect(ae.Spec.Approval.Namespace).To(Equal(testNamespace)) + }, timeout, interval).Should(Succeed()) + }) + + It("should delete ApprovalExpiration when Approval transitions to REJECTED", func() { + By("transitioning Approval to REJECTED") + approval := &approvalv1.Approval{} + Expect(k8sClient.Get(ctx, typeNamespacedName, approval)).To(Succeed()) + + approval.Spec.State = approvalv1.ApprovalStateRejected + approval.Spec.Decisions = append(approval.Spec.Decisions, approvalv1.Decision{ + Name: "Bob", + Email: "bob@telekom.de", + Comment: "Rejected", + ResultingState: approvalv1.ApprovalStateRejected, + }) + Expect(k8sClient.Update(ctx, approval)).To(Succeed()) + + expirationName := types.NamespacedName{ + Name: resourceName + "--expiration", + Namespace: testNamespace, + } + + By("checking ApprovalExpiration is deleted") + Eventually(func(g Gomega) { + ae := &approvalv1.ApprovalExpiration{} + err := k8sClient.Get(ctx, expirationName, ae) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + }, timeout, interval).Should(Succeed()) + }) + + It("should recreate ApprovalExpiration when Approval transitions back to GRANTED", func() { + By("transitioning Approval back to GRANTED") + approval := &approvalv1.Approval{} + Expect(k8sClient.Get(ctx, typeNamespacedName, approval)).To(Succeed()) + + approval.Spec.State = approvalv1.ApprovalStateGranted + approval.Spec.Decisions = append(approval.Spec.Decisions, approvalv1.Decision{ + Name: "Charlie", + Email: "charlie@telekom.de", + Comment: "Re-approved", + ResultingState: approvalv1.ApprovalStateGranted, + }) + Expect(k8sClient.Update(ctx, approval)).To(Succeed()) + + expirationName := types.NamespacedName{ + Name: resourceName + "--expiration", + Namespace: testNamespace, + } + + By("checking ApprovalExpiration is recreated") + Eventually(func(g Gomega) { + ae := &approvalv1.ApprovalExpiration{} + err := k8sClient.Get(ctx, expirationName, ae) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(ae.Spec.Expiration.Time).To(BeTemporally(">", time.Now())) + }, timeout, interval).Should(Succeed()) + }) + + It("should keep ApprovalExpiration when Approval transitions to SUSPENDED", func() { + expirationName := types.NamespacedName{ + Name: resourceName + "--expiration", + Namespace: testNamespace, + } + + By("getting current expiration time before SUSPENDED") + ae := &approvalv1.ApprovalExpiration{} + Expect(k8sClient.Get(ctx, expirationName, ae)).To(Succeed()) + originalExpiration := ae.Spec.Expiration.Time + + By("transitioning Approval to SUSPENDED") + approval := &approvalv1.Approval{} + Expect(k8sClient.Get(ctx, typeNamespacedName, approval)).To(Succeed()) + + approval.Spec.State = approvalv1.ApprovalStateSuspended + approval.Spec.Decisions = append(approval.Spec.Decisions, approvalv1.Decision{ + Name: "Dave", + Email: "dave@telekom.de", + Comment: "Suspended", + ResultingState: approvalv1.ApprovalStateSuspended, + }) + Expect(k8sClient.Update(ctx, approval)).To(Succeed()) + + By("checking ApprovalExpiration still exists with same expiration time") + Consistently(func(g Gomega) { + ae := &approvalv1.ApprovalExpiration{} + err := k8sClient.Get(ctx, expirationName, ae) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(ae.Spec.Expiration.Time).To(Equal(originalExpiration)) + }, 2*time.Second, interval).Should(Succeed()) + }) +}) diff --git a/approval/internal/controller/suite_test.go b/approval/internal/controller/suite_test.go index d8d009f5e..75bae8b24 100644 --- a/approval/internal/controller/suite_test.go +++ b/approval/internal/controller/suite_test.go @@ -24,6 +24,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/metrics/server" approvalv1 "github.com/telekom/controlplane/approval/api/v1" + "github.com/telekom/controlplane/approval/internal/config" + "github.com/telekom/controlplane/common/pkg/reminder" "github.com/telekom/controlplane/common/pkg/test" "github.com/telekom/controlplane/common/pkg/test/mock" "github.com/telekom/controlplane/common/pkg/test/testutil" @@ -120,15 +122,36 @@ var _ = BeforeSuite(func() { }) Expect(err).ToNot(HaveOccurred()) + // Create test expiration config (needed by ApprovalReconciler and ApprovalExpirationReconciler) + weeklyBefore := 30 * 24 * time.Hour + dailyBefore := 7 * 24 * time.Hour + dailyRepeat := 24 * time.Hour + + expirationConfig := &config.ExpirationConfig{ + ExpirationDuration: 12 * 30 * 24 * time.Hour, // 12 months + DefaultThresholds: []reminder.Threshold{ + {Before: metav1.Duration{Duration: weeklyBefore}}, + {Before: metav1.Duration{Duration: dailyBefore}, Repeat: &metav1.Duration{Duration: dailyRepeat}}, + }, + } + err = (&ApprovalReconciler{ + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), + Recorder: &mock.EventRecorder{}, + ExpirationConfig: expirationConfig, + }).SetupWithManager(k8sManager) + + Expect(err).ToNot(HaveOccurred()) + + err = (&ApprovalRequestReconciler{ Client: k8sManager.GetClient(), Scheme: k8sManager.GetScheme(), Recorder: &mock.EventRecorder{}, }).SetupWithManager(k8sManager) - Expect(err).ToNot(HaveOccurred()) - err = (&ApprovalRequestReconciler{ + err = (&ApprovalExpirationReconciler{ Client: k8sManager.GetClient(), Scheme: k8sManager.GetScheme(), Recorder: &mock.EventRecorder{}, diff --git a/approval/internal/handler/approval/fsm.go b/approval/internal/handler/approval/fsm.go index 5be527cb8..8d879fbea 100644 --- a/approval/internal/handler/approval/fsm.go +++ b/approval/internal/handler/approval/fsm.go @@ -10,25 +10,29 @@ import ( ) var auto = fsm.Transitions{ - {Action: v1.ApprovalActionAllow, Src: []v1.ApprovalState{v1.ApprovalStateRejected}, Dst: v1.ApprovalStateGranted}, - {Action: v1.ApprovalActionDeny, Src: []v1.ApprovalState{v1.ApprovalStateGranted}, Dst: v1.ApprovalStateRejected}, + {Action: v1.ApprovalActionAllow, Src: []v1.ApprovalState{v1.ApprovalStateRejected, v1.ApprovalStateExpired}, Dst: v1.ApprovalStateGranted}, + {Action: v1.ApprovalActionDeny, Src: []v1.ApprovalState{v1.ApprovalStateGranted, v1.ApprovalStateExpired}, Dst: v1.ApprovalStateRejected}, {Action: v1.ApprovalActionSuspend, Src: []v1.ApprovalState{v1.ApprovalStateGranted}, Dst: v1.ApprovalStateSuspended}, {Action: v1.ApprovalActionResume, Src: []v1.ApprovalState{v1.ApprovalStateSuspended}, Dst: v1.ApprovalStateGranted}, + {Action: v1.ApprovalActionExpire, Src: []v1.ApprovalState{v1.ApprovalStateGranted, v1.ApprovalStateSuspended}, Dst: v1.ApprovalStateExpired}, } var simple = fsm.Transitions{ - {Action: v1.ApprovalActionAllow, Src: []v1.ApprovalState{v1.ApprovalStatePending, v1.ApprovalStateRejected}, Dst: v1.ApprovalStateGranted}, - {Action: v1.ApprovalActionDeny, Src: []v1.ApprovalState{v1.ApprovalStatePending, v1.ApprovalStateGranted}, Dst: v1.ApprovalStateRejected}, + {Action: v1.ApprovalActionAllow, Src: []v1.ApprovalState{v1.ApprovalStatePending, v1.ApprovalStateRejected, v1.ApprovalStateExpired}, Dst: v1.ApprovalStateGranted}, + {Action: v1.ApprovalActionDeny, Src: []v1.ApprovalState{v1.ApprovalStatePending, v1.ApprovalStateGranted, v1.ApprovalStateExpired}, Dst: v1.ApprovalStateRejected}, {Action: v1.ApprovalActionSuspend, Src: []v1.ApprovalState{v1.ApprovalStateGranted}, Dst: v1.ApprovalStateSuspended}, {Action: v1.ApprovalActionResume, Src: []v1.ApprovalState{v1.ApprovalStateSuspended}, Dst: v1.ApprovalStateGranted}, + {Action: v1.ApprovalActionExpire, Src: []v1.ApprovalState{v1.ApprovalStateGranted, v1.ApprovalStateSuspended}, Dst: v1.ApprovalStateExpired}, } var fourEyes = fsm.Transitions{ {Action: v1.ApprovalActionAllow, Src: []v1.ApprovalState{v1.ApprovalStatePending, v1.ApprovalStateRejected}, Dst: v1.ApprovalStateSemigranted}, - {Action: v1.ApprovalActionDeny, Src: []v1.ApprovalState{v1.ApprovalStatePending, v1.ApprovalStateGranted, v1.ApprovalStateSemigranted}, Dst: v1.ApprovalStateRejected}, {Action: v1.ApprovalActionAllow, Src: []v1.ApprovalState{v1.ApprovalStateSemigranted}, Dst: v1.ApprovalStateGranted}, + {Action: v1.ApprovalActionAllow, Src: []v1.ApprovalState{v1.ApprovalStateExpired}, Dst: v1.ApprovalStateGranted}, + {Action: v1.ApprovalActionDeny, Src: []v1.ApprovalState{v1.ApprovalStatePending, v1.ApprovalStateGranted, v1.ApprovalStateSemigranted, v1.ApprovalStateExpired}, Dst: v1.ApprovalStateRejected}, {Action: v1.ApprovalActionSuspend, Src: []v1.ApprovalState{v1.ApprovalStateGranted}, Dst: v1.ApprovalStateSuspended}, {Action: v1.ApprovalActionResume, Src: []v1.ApprovalState{v1.ApprovalStateSuspended}, Dst: v1.ApprovalStateGranted}, + {Action: v1.ApprovalActionExpire, Src: []v1.ApprovalState{v1.ApprovalStateGranted, v1.ApprovalStateSuspended}, Dst: v1.ApprovalStateExpired}, } var transitionMap = map[v1.ApprovalStrategy]fsm.Transitions{ diff --git a/approval/internal/handler/approval/handler.go b/approval/internal/handler/approval/handler.go index 46fd4f951..3f0f61855 100644 --- a/approval/internal/handler/approval/handler.go +++ b/approval/internal/handler/approval/handler.go @@ -6,21 +6,38 @@ package approval import ( "context" + "fmt" + "time" "github.com/pkg/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" approvalv1 "github.com/telekom/controlplane/approval/api/v1" approval_condition "github.com/telekom/controlplane/approval/internal/condition" + "github.com/telekom/controlplane/approval/internal/config" "github.com/telekom/controlplane/approval/internal/handler/util" + commonclient "github.com/telekom/controlplane/common/pkg/client" "github.com/telekom/controlplane/common/pkg/condition" "github.com/telekom/controlplane/common/pkg/handler" + "github.com/telekom/controlplane/common/pkg/types" "github.com/telekom/controlplane/common/pkg/util/contextutil" ) var _ handler.Handler[*approvalv1.Approval] = &ApprovalHandler{} -type ApprovalHandler struct{} +type ApprovalHandler struct { + cfg *config.ExpirationConfig +} + +// NewHandler creates a new ApprovalHandler with the provided configuration +func NewHandler(client client.Client, cfg *config.ExpirationConfig) *ApprovalHandler { + return &ApprovalHandler{ + cfg: cfg, + } +} func (h *ApprovalHandler) CreateOrUpdate(ctx context.Context, approval *approvalv1.Approval) error { // handle the notifications first @@ -31,7 +48,11 @@ func (h *ApprovalHandler) CreateOrUpdate(ctx context.Context, approval *approval } fsm := ApprovalStrategyFSM[approval.Spec.Strategy] - approval.Status.AvailableTransitions = fsm.AvailableTransitions(approval.Spec.State) + // Filter out system-only actions (Expire) from available transitions + approval.Status.AvailableTransitions = filterSystemActions(fsm.AvailableTransitions(approval.Spec.State)) + + // Capture state change BEFORE updating LastState (needed for expiration logic) + stateChanged := approval.Spec.State != approval.Status.LastState approval.Status.LastState = approval.Spec.State switch approval.Spec.State { @@ -60,6 +81,16 @@ func (h *ApprovalHandler) CreateOrUpdate(ctx context.Context, approval *approval approval.SetCondition(condition.NewProcessingCondition("Suspended", "Approval is suspended")) approval.SetCondition(condition.NewReadyCondition("Suspended", "Approval is suspended")) + case approvalv1.ApprovalStateExpired: + approval.SetCondition(approval_condition.NewExpiredCondition()) + approval.SetCondition(condition.NewProcessingCondition("Expired", "Approval has expired")) + approval.SetCondition(condition.NewReadyCondition("Expired", "Approval has expired but can be re-approved")) + + } + + // Handle ApprovalExpiration lifecycle (do this after conditions are set) + if err := h.handleExpiration(ctx, approval, stateChanged); err != nil { + return errors.Wrap(err, "failed to handle expiration") } return nil @@ -125,3 +156,112 @@ func (h *ApprovalHandler) Delete(ctx context.Context, approval *approvalv1.Appro logger.Info("Approval deleted") return nil } + +// filterSystemActions removes system-only actions (Expire) from the available transitions +func filterSystemActions(transitions approvalv1.AvailableTransitions) approvalv1.AvailableTransitions { + var filtered approvalv1.AvailableTransitions + for _, t := range transitions { + if t.Action != approvalv1.ApprovalActionExpire { + filtered = append(filtered, t) + } + } + return filtered +} + +// handleExpiration manages the lifecycle of ApprovalExpiration resources +func (h *ApprovalHandler) handleExpiration(ctx context.Context, approval *approvalv1.Approval, stateChanged bool) error { + // Only for non-Auto strategies + if approval.Spec.Strategy == approvalv1.ApprovalStrategyAuto { + return nil + } + + c := commonclient.ClientFromContextOrDie(ctx) + + switch approval.Spec.State { + case approvalv1.ApprovalStateGranted: + if stateChanged { + // Create or update ApprovalExpiration with fresh dates + // (covers initial GRANTED and REALLOW from EXPIRED) + return createOrUpdateApprovalExpiration(ctx, c, approval, h.cfg) + } + // State unchanged, leave ApprovalExpiration alone + + case approvalv1.ApprovalStateSuspended: + // Clock keeps ticking, leave ApprovalExpiration alone + + case approvalv1.ApprovalStateExpired: + // Already expired, leave ApprovalExpiration alone + + case approvalv1.ApprovalStateRejected: + if stateChanged { + // Delete ApprovalExpiration + return deleteApprovalExpiration(ctx, c, approval) + } + + case approvalv1.ApprovalStatePending, approvalv1.ApprovalStateSemigranted: + // No ApprovalExpiration should exist in these states + // (safety: delete if exists, but shouldn't happen) + return deleteApprovalExpiration(ctx, c, approval) + } + + return nil +} + +// createOrUpdateApprovalExpiration creates or updates an ApprovalExpiration with fresh dates +func createOrUpdateApprovalExpiration(ctx context.Context, c commonclient.JanitorClient, approval *approvalv1.Approval, cfg *config.ExpirationConfig) error { + now := time.Now() + expirationDate := now.Add(cfg.ExpirationDuration) + + ae := &approvalv1.ApprovalExpiration{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s--expiration", approval.Name), + Namespace: approval.Namespace, + }, + } + + mutate := func() error { + if err := controllerutil.SetControllerReference(approval, ae, c.Scheme()); err != nil { + return errors.Wrap(err, "failed to set controller reference") + } + ae.Spec = approvalv1.ApprovalExpirationSpec{ + Approval: types.ObjectRef{ + Name: approval.Name, + Namespace: approval.Namespace, + }, + Expiration: metav1.Time{Time: expirationDate}, + Thresholds: cfg.DefaultThresholds, + } + return nil + } + + _, err := c.CreateOrUpdate(ctx, ae, mutate) + if err != nil { + return errors.Wrap(err, "failed to create or update ApprovalExpiration") + } + + log.FromContext(ctx).Info("Created or updated ApprovalExpiration", "name", ae.Name, "expiration", expirationDate) + return nil +} + +// deleteApprovalExpiration deletes the ApprovalExpiration if it exists +func deleteApprovalExpiration(ctx context.Context, c commonclient.JanitorClient, approval *approvalv1.Approval) error { + ae := &approvalv1.ApprovalExpiration{} + key := client.ObjectKey{ + Name: fmt.Sprintf("%s--expiration", approval.Name), + Namespace: approval.Namespace, + } + + err := c.Get(ctx, key, ae) + if err != nil { + // Doesn't exist, nothing to delete + return client.IgnoreNotFound(err) + } + + // Exists, delete it + if err := c.Delete(ctx, ae); err != nil { + return errors.Wrap(err, "failed to delete ApprovalExpiration") + } + + log.FromContext(ctx).Info("Deleted ApprovalExpiration", "name", ae.Name) + return nil +} diff --git a/approval/internal/handler/approvalexpiration/handler.go b/approval/internal/handler/approvalexpiration/handler.go new file mode 100644 index 000000000..813e455e7 --- /dev/null +++ b/approval/internal/handler/approvalexpiration/handler.go @@ -0,0 +1,189 @@ +// Copyright 2025 Deutsche Telekom IT GmbH +// +// SPDX-License-Identifier: Apache-2.0 + +package approvalexpiration + +import ( + "context" + "fmt" + "time" + + "github.com/pkg/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + v1 "github.com/telekom/controlplane/approval/api/v1" + "github.com/telekom/controlplane/approval/internal/handler/util" + "github.com/telekom/controlplane/common/pkg/reminder" + "github.com/telekom/controlplane/common/pkg/types" + "github.com/telekom/controlplane/common/pkg/util/contextutil" +) + +// Handler handles ApprovalExpiration resources +type Handler struct { + client client.Client +} + +// NewHandler creates a new ApprovalExpiration handler +func NewHandler(c client.Client) *Handler { + return &Handler{ + client: c, + } +} + +// CreateOrUpdate processes an ApprovalExpiration resource +func (h *Handler) CreateOrUpdate(ctx context.Context, ae *v1.ApprovalExpiration) error { + logger := log.FromContext(ctx) + + // Load parent Approval + approval, err := h.getParentApproval(ctx, ae) + if err != nil { + return errors.Wrap(err, "failed to get parent approval") + } + + // Guard: parent must be in active state (GRANTED or SUSPENDED) + if approval.Spec.State != v1.ApprovalStateGranted && approval.Spec.State != v1.ApprovalStateSuspended { + logger.V(1).Info("Parent approval not in expirable state, skipping", + "approvalState", approval.Spec.State) + return nil + } + + now := time.Now() + + // Check if expired — transition to EXPIRED and return immediately. + // No notification is sent here because the Approval handler's handleNotifications + // already sends state-change notifications when it processes the GRANTED→EXPIRED transition. + if now.After(ae.Spec.Expiration.Time) || now.Equal(ae.Spec.Expiration.Time) { + logger.Info("Approval expired, transitioning to EXPIRED state", + "expiration", ae.Spec.Expiration.Time) + return h.ensureApprovalExpired(ctx, approval) + } + + // Evaluate which reminder (if any) should fire now + pending := reminder.Evaluate(ae.Spec.Expiration.Time, ae.Spec.Thresholds, ae.Status.SentReminders, now) + if len(pending) > 0 { + p := pending[0] + logger.V(1).Info("Sending reminder", "threshold", p.Key) + + // Send notification + notificationRef, err := h.sendReminder(ctx, ae, approval, now, p.Threshold) + if err != nil { + return errors.Wrap(err, "failed to send reminder") + } + + // Track that this reminder was sent + ae.Status.SentReminders = reminder.UpsertSent(ae.Status.SentReminders, &reminder.SentReminder{ + Threshold: p.Key, + Ref: *notificationRef, + SentAt: metav1.NewTime(now), + }) + } + + // Schedule next reconciliation via the happy-path requeue mechanism. + // We use SetRequeueAfter instead of returning an error to avoid the common + // controller marking the object as NotReady and emitting Warning events. + delay := reminder.NextRequeue(ae.Spec.Expiration.Time, ae.Spec.Thresholds, ae.Status.SentReminders, now) + if delay <= 0 { + delay = time.Second // Immediate retry + } + logger.V(1).Info("Scheduling next expiration check", "requeueAfter", delay) + contextutil.SetRequeueAfter(ctx, delay) + return nil +} + +// Delete handles cleanup when an ApprovalExpiration is deleted +func (h *Handler) Delete(ctx context.Context, ae *v1.ApprovalExpiration) error { + // Notifications are owned by ApprovalExpiration, auto-deleted by GC + return nil +} + +// getParentApproval fetches the parent Approval resource +func (h *Handler) getParentApproval(ctx context.Context, ae *v1.ApprovalExpiration) (*v1.Approval, error) { + approval := &v1.Approval{} + key := client.ObjectKey{ + Name: ae.Spec.Approval.Name, + Namespace: ae.Spec.Approval.Namespace, + } + if err := h.client.Get(ctx, key, approval); err != nil { + return nil, err + } + return approval, nil +} + +// ensureApprovalExpired transitions the parent Approval to EXPIRED state +func (h *Handler) ensureApprovalExpired(ctx context.Context, approval *v1.Approval) error { + if approval.Spec.State == v1.ApprovalStateExpired { + return nil // Already expired, no-op + } + + // Add system Decision (matches auto-approval pattern) + approval.Spec.Decisions = append(approval.Spec.Decisions, v1.Decision{ + Name: v1.SystemDecisionName, + Comment: "Automatically expired - expiration date reached. Available actions: Allow (re-approve), Deny (reject).", + Timestamp: &metav1.Time{Time: time.Now()}, + ResultingState: v1.ApprovalStateExpired, + }) + + // Set state + approval.Spec.State = v1.ApprovalStateExpired + + // Update via API server (goes through webhook) + if err := h.client.Update(ctx, approval); err != nil { + return errors.Wrap(err, "failed to expire approval") + } + + return nil +} + +// sendReminder sends a reminder notification for the approaching or reached expiration +func (h *Handler) sendReminder(ctx context.Context, ae *v1.ApprovalExpiration, approval *v1.Approval, now time.Time, threshold reminder.Threshold) (*types.ObjectRef, error) { + // Calculate days remaining + daysRemaining := int64(ae.Spec.Expiration.Time.Sub(now).Hours() / 24) + if daysRemaining < 0 { + daysRemaining = 0 + } + + // Determine reminder type based on threshold + var reminderType string + if threshold.Repeat != nil { + reminderType = "daily" // Repeating thresholds are daily + } else if daysRemaining == 0 { + reminderType = "expired" + } else { + reminderType = "weekly" // One-shot thresholds are weekly + } + + // Build base notification data with template properties + notificationData := util.NotificationData{ + Owner: ae, + StateNew: string(approval.Spec.State), + StateOld: string(approval.Spec.State), // State hasn't changed yet for reminders + Target: &approval.Spec.Target, + Requester: &approval.Spec.Requester, + Decider: &approval.Spec.Decider, + Scenario: util.NotificationScenarioUpdated, + Action: approval.Spec.Action, + ExpirationDate: ae.Spec.Expiration.Format("2006-01-02"), + DaysRemaining: fmt.Sprintf("%d", daysRemaining), + ReminderType: reminderType, + } + + // Send to decider (in decider's namespace) + notificationData.Actor = util.ActorDecider + notificationData.SendToChannelNamespace = approval.Spec.Decider.ApplicationRef.Namespace + if _, err := util.SendReminderNotification(ctx, ¬ificationData); err != nil { + return nil, errors.Wrap(err, "failed to send decider reminder") + } + + // Send to requester (in requester's namespace) + notificationData.Actor = util.ActorRequester + notificationData.SendToChannelNamespace = approval.Spec.Requester.ApplicationRef.Namespace + requesterRef, err := util.SendReminderNotification(ctx, ¬ificationData) + if err != nil { + return nil, errors.Wrap(err, "failed to send requester reminder") + } + + return requesterRef, nil +} diff --git a/approval/internal/handler/approvalexpiration/handler_test.go b/approval/internal/handler/approvalexpiration/handler_test.go new file mode 100644 index 000000000..301d235dd --- /dev/null +++ b/approval/internal/handler/approvalexpiration/handler_test.go @@ -0,0 +1,76 @@ +// Copyright 2025 Deutsche Telekom IT GmbH +// +// SPDX-License-Identifier: Apache-2.0 + +package approvalexpiration + +import ( + "testing" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + v1 "github.com/telekom/controlplane/approval/api/v1" + "github.com/telekom/controlplane/approval/internal/config" + "github.com/telekom/controlplane/common/pkg/reminder" +) + +func TestApprovalExpirationHandler(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "ApprovalExpiration Handler Suite") +} + +var _ = Describe("ApprovalExpiration Handler", func() { + var cfg *config.ExpirationConfig + + BeforeEach(func() { + // Create config with default thresholds + weeklyBefore := 30 * 24 * time.Hour + dailyBefore := 7 * 24 * time.Hour + dailyRepeat := 24 * time.Hour + + cfg = &config.ExpirationConfig{ + ExpirationDuration: 12 * 30 * 24 * time.Hour, // 12 months + DefaultThresholds: []reminder.Threshold{ + {Before: metav1.Duration{Duration: weeklyBefore}}, + {Before: metav1.Duration{Duration: dailyBefore}, Repeat: &metav1.Duration{Duration: dailyRepeat}}, + }, + } + }) + + Describe("config integration", func() { + It("should have valid default thresholds", func() { + Expect(cfg.DefaultThresholds).To(HaveLen(2)) + Expect(cfg.DefaultThresholds[0].Before.Duration).To(Equal(30 * 24 * time.Hour)) + Expect(cfg.DefaultThresholds[0].Repeat).To(BeNil()) + Expect(cfg.DefaultThresholds[1].Before.Duration).To(Equal(7 * 24 * time.Hour)) + Expect(cfg.DefaultThresholds[1].Repeat).NotTo(BeNil()) + Expect(cfg.DefaultThresholds[1].Repeat.Duration).To(Equal(24 * time.Hour)) + }) + }) + + Describe("reminder evaluation", func() { + It("should use common/pkg/reminder for scheduling", func() { + now := time.Date(2026, 5, 1, 12, 0, 0, 0, time.UTC) + expiration := now.Add(10 * 24 * time.Hour) // 10 days out + + ae := &v1.ApprovalExpiration{ + Spec: v1.ApprovalExpirationSpec{ + Expiration: metav1.Time{Time: expiration}, + Thresholds: cfg.DefaultThresholds, + }, + Status: v1.ApprovalExpirationStatus{ + SentReminders: []reminder.SentReminder{}, + }, + } + + // 10 days out: we're in the weekly window (30 days before) but not yet in daily window (7 days before) + // reminder.Evaluate returns the tightest matching threshold + pending := reminder.Evaluate(ae.Spec.Expiration.Time, ae.Spec.Thresholds, ae.Status.SentReminders, now) + Expect(pending).To(HaveLen(1)) + Expect(pending[0].Key).To(Equal("720h0m0s")) // 30 days (weekly threshold) + }) + }) +}) diff --git a/approval/internal/handler/util/notification.go b/approval/internal/handler/util/notification.go index 33a146293..a99be4b03 100644 --- a/approval/internal/handler/util/notification.go +++ b/approval/internal/handler/util/notification.go @@ -45,9 +45,12 @@ const ( // TemplatePlaceholderResourceType can be api/event TemplatePlaceholderResourceType = "resource_type" - TemplatePlaceholderStateOld = "state_old" - TemplatePlaceholderStateNew = "state_new" - TemplatePlaceholderScopes = "scopes" + TemplatePlaceholderStateOld = "state_old" + TemplatePlaceholderStateNew = "state_new" + TemplatePlaceholderScopes = "scopes" + TemplatePlaceholderExpirationDate = "expiration_date" + TemplatePlaceholderDaysRemaining = "days_remaining" + TemplatePlaceholderReminderType = "reminder_type" ) const ( @@ -78,6 +81,10 @@ type NotificationData struct { Scenario NotificationScenario Actor Actor Action string + // Expiration-specific fields + ExpirationDate string + DaysRemaining string + ReminderType string } func extractDecider(decider *approvalv1.Decider) (map[string]any, error) { @@ -226,6 +233,69 @@ func initializeProperties() map[string]any { properties[TemplatePlaceholderStateOld] = defaultValue properties[TemplatePlaceholderStateNew] = defaultValue properties[TemplatePlaceholderScopes] = defaultValue + // Note: Expiration fields (ExpirationDate, DaysRemaining, ReminderType) are only + // initialized in SendReminderNotification, not for regular notifications return properties } + +// SendReminderNotification sends an expiration reminder notification +func SendReminderNotification(ctx context.Context, data *NotificationData) (*types.ObjectRef, error) { + properties := initializeProperties() + + properties[TemplatePlaceholderEnvironment] = contextutil.EnvFromContextOrDie(ctx) + properties[TemplatePlaceholderStateNew] = data.StateNew + properties[TemplatePlaceholderStateOld] = data.StateOld + properties[TemplatePlaceholderExpirationDate] = data.ExpirationDate + properties[TemplatePlaceholderDaysRemaining] = data.DaysRemaining + properties[TemplatePlaceholderReminderType] = data.ReminderType + + requesterMap, err := extractRequester(data.Requester) + if err != nil { + return nil, errors.Wrap(err, "failed to extract requester data") + } + for k, v := range requesterMap { + properties[strings.ToLower(k)] = v + } + + deciderMap, err := extractDecider(data.Decider) + if err != nil { + return nil, errors.Wrap(err, "failed to extract decider data") + } + for k, v := range deciderMap { + properties[strings.ToLower(k)] = v + } + + // Build purpose: ----reminder-- + // Example: approvalexpiration--subscribe--reminder--decider + purposeStringBuilder := strings.Builder{} + purposeStringBuilder.WriteString(strings.ToLower(data.Owner.GetObjectKind().GroupVersionKind().Kind)) + purposeStringBuilder.WriteString(DELIMITER) + purposeStringBuilder.WriteString(data.Action) + purposeStringBuilder.WriteString(DELIMITER) + purposeStringBuilder.WriteString("reminder") + purposeStringBuilder.WriteString(DELIMITER) + purposeStringBuilder.WriteString(string(data.Actor)) + purpose := purposeStringBuilder.String() + + // Build notification name: ---- + nameStringBuilder := strings.Builder{} + nameStringBuilder.WriteString(purpose) + nameStringBuilder.WriteString(DELIMITER) + nameStringBuilder.WriteString(data.Target.GetName()) + name := nameStringBuilder.String() + + notificationBuilder := builder.New(). + WithOwner(data.Owner). + WithSender(notificationv1.SenderTypeSystem, "ApprovalService"). + WithDefaultChannels(ctx, data.SendToChannelNamespace). + WithPurpose(strings.ToLower(purpose)). + WithName(labelutil.NormalizeNameValue(name)). + WithProperties(properties) + + notification, err := notificationBuilder.Send(ctx) + if err != nil { + return nil, err + } + return types.ObjectRefFromObject(notification), nil +} diff --git a/approval/internal/webhook/v1/approval_webhook.go b/approval/internal/webhook/v1/approval_webhook.go index 00cea17ef..7fca42266 100644 --- a/approval/internal/webhook/v1/approval_webhook.go +++ b/approval/internal/webhook/v1/approval_webhook.go @@ -6,6 +6,7 @@ package v1 import ( "context" + "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" @@ -73,7 +74,7 @@ func (a *ApprovalCustomValidator) ValidateCreate(_ context.Context, obj *approva } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type -func (a *ApprovalCustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj *approvalv1.Approval) (warnings admission.Warnings, err error) { +func (a *ApprovalCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj *approvalv1.Approval) (warnings admission.Warnings, err error) { approvallog.Info("validate update", "name", newObj.Name) if newObj.Spec.Strategy == approvalv1.ApprovalStrategyAuto && newObj.Spec.State != approvalv1.ApprovalStateGranted { @@ -86,6 +87,10 @@ func (a *ApprovalCustomValidator) ValidateUpdate(_ context.Context, oldObj, newO // instead of Status.AvailableTransitions (which may be stale or nil before // the controller has reconciled). Auto strategy uses its own FSM. if stateChanged { + if validationErr := validateExpireTransition(ctx, newObj); validationErr != nil { + return warnings, validationErr + } + fsmDef, ok := approvalhandler.ApprovalStrategyFSM[newObj.Spec.Strategy] if !ok { err = apierrors.NewBadRequest("Unknown approval strategy") @@ -124,3 +129,29 @@ func (a *ApprovalCustomValidator) ValidateDelete(_ context.Context, obj *approva return nil, nil } + +// controllerServiceAccountSuffix is the suffix of the service account used by the approval controller. +// The full username is "system:serviceaccount::controller-manager". +const controllerServiceAccountSuffix = "controller-manager" + +// validateExpireTransition blocks manual transitions to EXPIRED state. +// Only the controller service account is allowed to perform this transition. +func validateExpireTransition(ctx context.Context, newObj *approvalv1.Approval) error { + if newObj.Spec.State != approvalv1.ApprovalStateExpired { + return nil + } + + // Verify the caller is the controller service account + req, err := admission.RequestFromContext(ctx) + if err != nil { + return apierrors.NewBadRequest("Expire action is system-only and cannot be triggered manually") + } + + username := req.UserInfo.Username + // Expected format: system:serviceaccount::controller-manager + if strings.HasPrefix(username, "system:serviceaccount:") && strings.HasSuffix(username, controllerServiceAccountSuffix) { + return nil + } + + return apierrors.NewBadRequest("Expire action is system-only and cannot be triggered manually") +} diff --git a/approval/internal/webhook/v1/approval_webhook_test.go b/approval/internal/webhook/v1/approval_webhook_test.go index 8d3776dea..9cb98d0c0 100644 --- a/approval/internal/webhook/v1/approval_webhook_test.go +++ b/approval/internal/webhook/v1/approval_webhook_test.go @@ -8,7 +8,10 @@ import ( "context" "time" + admissionv1 "k8s.io/api/admission/v1" + authenticationv1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" approvalv1 "github.com/telekom/controlplane/approval/api/v1" @@ -16,6 +19,18 @@ import ( . "github.com/onsi/gomega" ) +// controllerContext returns a context that simulates a request from the controller service account. +func controllerContext() context.Context { + req := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + UserInfo: authenticationv1.UserInfo{ + Username: "system:serviceaccount:controlplane-system:approval-controller-manager", + }, + }, + } + return admission.NewContextWithRequest(context.Background(), req) +} + var _ = Describe("Approval Webhook", func() { Context("When creating Approval under Defaulting Webhook", func() { It("Should not modify non-Auto strategy approvals", func() { @@ -473,4 +488,80 @@ var _ = Describe("Approval Webhook", func() { Expect(err).NotTo(HaveOccurred()) }) }) + + Context("Expire action validation (system-only)", func() { + var validator ApprovalCustomValidator + + makeApproval := func(specState approvalv1.ApprovalState, decisions []approvalv1.Decision) *approvalv1.Approval { + return &approvalv1.Approval{ + Spec: approvalv1.ApprovalSpec{ + Strategy: approvalv1.ApprovalStrategySimple, + State: specState, + Decisions: decisions, + }, + } + } + + It("should reject manual transition to EXPIRED without System decision", func() { + oldObj := makeApproval(approvalv1.ApprovalStateGranted, nil) + newObj := makeApproval(approvalv1.ApprovalStateExpired, []approvalv1.Decision{ + {Name: "Alice", Email: "alice@telekom.de", Comment: "Manual expire", ResultingState: approvalv1.ApprovalStateExpired}, + }) + _, err := validator.ValidateUpdate(context.Background(), oldObj, newObj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Expire action is system-only")) + }) + + It("should reject manual transition to EXPIRED from SUSPENDED without System decision", func() { + oldObj := makeApproval(approvalv1.ApprovalStateSuspended, nil) + newObj := makeApproval(approvalv1.ApprovalStateExpired, []approvalv1.Decision{ + {Name: "Bob", Email: "bob@telekom.de", Comment: "Manual expire", ResultingState: approvalv1.ApprovalStateExpired}, + }) + _, err := validator.ValidateUpdate(context.Background(), oldObj, newObj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Expire action is system-only")) + }) + + It("should allow controller service account to transition to EXPIRED", func() { + oldObj := makeApproval(approvalv1.ApprovalStateGranted, nil) + newObj := makeApproval(approvalv1.ApprovalStateExpired, []approvalv1.Decision{ + {Name: "System", Email: "", Comment: "Automatically expired", ResultingState: approvalv1.ApprovalStateExpired}, + }) + _, err := validator.ValidateUpdate(controllerContext(), oldObj, newObj) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should allow controller service account to transition to EXPIRED from SUSPENDED", func() { + oldObj := makeApproval(approvalv1.ApprovalStateSuspended, nil) + newObj := makeApproval(approvalv1.ApprovalStateExpired, []approvalv1.Decision{ + {Name: "System", Email: "", Comment: "Automatically expired", ResultingState: approvalv1.ApprovalStateExpired}, + }) + _, err := validator.ValidateUpdate(controllerContext(), oldObj, newObj) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should allow transition from EXPIRED to GRANTED (re-approval)", func() { + oldObj := makeApproval(approvalv1.ApprovalStateExpired, []approvalv1.Decision{ + {Name: "System", Email: "", Comment: "Automatically expired", ResultingState: approvalv1.ApprovalStateExpired}, + }) + newObj := makeApproval(approvalv1.ApprovalStateGranted, []approvalv1.Decision{ + {Name: "System", Email: "", Comment: "Automatically expired", ResultingState: approvalv1.ApprovalStateExpired}, + {Name: "Charlie", Email: "charlie@telekom.de", Comment: "Re-approved", ResultingState: approvalv1.ApprovalStateGranted}, + }) + _, err := validator.ValidateUpdate(context.Background(), oldObj, newObj) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should allow transition from EXPIRED to REJECTED", func() { + oldObj := makeApproval(approvalv1.ApprovalStateExpired, []approvalv1.Decision{ + {Name: "System", Email: "", Comment: "Automatically expired", ResultingState: approvalv1.ApprovalStateExpired}, + }) + newObj := makeApproval(approvalv1.ApprovalStateRejected, []approvalv1.Decision{ + {Name: "System", Email: "", Comment: "Automatically expired", ResultingState: approvalv1.ApprovalStateExpired}, + {Name: "Dave", Email: "dave@telekom.de", Comment: "Denied", ResultingState: approvalv1.ApprovalStateRejected}, + }) + _, err := validator.ValidateUpdate(context.Background(), oldObj, newObj) + Expect(err).NotTo(HaveOccurred()) + }) + }) }) diff --git a/approval/internal/webhook/v1/approvalrequest_webhook.go b/approval/internal/webhook/v1/approvalrequest_webhook.go index 5704125cb..cbc1c4773 100644 --- a/approval/internal/webhook/v1/approvalrequest_webhook.go +++ b/approval/internal/webhook/v1/approvalrequest_webhook.go @@ -52,7 +52,7 @@ func (ar *ApprovalRequestCustomDefaulter) Default(_ context.Context, obj *approv obj.Spec.State = approvalv1.ApprovalStateGranted if len(obj.Spec.Decisions) == 0 { obj.Spec.Decisions = append(obj.Spec.Decisions, approvalv1.Decision{ - Name: "System", + Name: approvalv1.SystemDecisionName, Comment: approvalv1.AutoApprovedComment, ResultingState: approvalv1.ApprovalStateGranted, })