Skip to content

Commit 751a5b9

Browse files
authored
[CHAOSPLT-463] Set user info annotation on disruption creation from parent resource (#927)
* fix: store user information in annotations * refactor and unit tests * nit * fix: log error when can't parse annotations * fix: unit tests
1 parent 5b4d7f3 commit 751a5b9

7 files changed

+273
-18
lines changed

api/v1beta1/disruption_types.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@ import (
1616
"os"
1717
"path/filepath"
1818
"reflect"
19+
"strings"
1920
"time"
2021

2122
chaosapi "github.com/DataDog/chaos-controller/api"
2223
eventtypes "github.com/DataDog/chaos-controller/eventnotifier/types"
2324
chaostypes "github.com/DataDog/chaos-controller/types"
2425
"github.com/DataDog/chaos-controller/utils"
2526
"github.com/hashicorp/go-multierror"
27+
authv1beta1 "k8s.io/api/authentication/v1beta1"
2628
corev1 "k8s.io/api/core/v1"
2729
apierrors "k8s.io/apimachinery/pkg/api/errors"
2830
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -448,6 +450,69 @@ func (r *Disruption) IsReadyToRemoveFinalizer(finalizerDelay time.Duration) bool
448450
return r.Status.CleanedAt != nil && time.Now().After(r.Status.CleanedAt.Add(finalizerDelay))
449451
}
450452

453+
// CopyOwnerAnnotations copies the annotations from the owner object to the disruption.
454+
// This ensures that any important metadata from the owner, such as custom annotations,
455+
// is preserved in the newly created disruption.
456+
func (r *Disruption) CopyOwnerAnnotations(owner metav1.Object) {
457+
if r.Annotations == nil {
458+
r.Annotations = make(map[string]string)
459+
}
460+
461+
ownerAnnotations := owner.GetAnnotations()
462+
for k, v := range ownerAnnotations {
463+
r.Annotations[k] = v
464+
}
465+
}
466+
467+
// SetScheduledAtAnnotation sets the scheduled time of the disruption in the annotations.
468+
func (r *Disruption) SetScheduledAtAnnotation(scheduledTime time.Time) {
469+
if r.Annotations == nil {
470+
r.Annotations = make(map[string]string)
471+
}
472+
473+
scheduledAt := scheduledTime.Format(time.RFC3339)
474+
r.Annotations[chaostypes.ScheduledAtAnnotation] = scheduledAt
475+
}
476+
477+
// GetScheduledAtAnnotation retrieves the scheduled time from the disruption's annotations.
478+
// Returns an error if the annotation is not found or cannot be parsed.
479+
func (r *Disruption) GetScheduledAtAnnotation() (time.Time, error) {
480+
scheduledAt, exists := r.Annotations[chaostypes.ScheduledAtAnnotation]
481+
if !exists {
482+
return time.Time{}, errors.New("scheduledAt annotation not found")
483+
}
484+
485+
scheduledTime, err := time.Parse(time.RFC3339, scheduledAt)
486+
if err != nil {
487+
return time.Time{}, fmt.Errorf("unable to parse scheduledAt annotation: %w", err)
488+
}
489+
490+
return scheduledTime, nil
491+
}
492+
493+
// CopyUserInfoToAnnotations copies the user-related annotations from the owner object to the disruption.
494+
// Any UserInfo annotations will be overwritten when the Disruption is created, so this function ensures
495+
// that the parent resource's user information is preserved by storing it in separate annotations.
496+
func (r *Disruption) CopyUserInfoToAnnotations(owner metav1.Object) error {
497+
if r.Annotations == nil {
498+
r.Annotations = make(map[string]string)
499+
}
500+
501+
ownerAnnotations := owner.GetAnnotations()
502+
if userInfoJSON, exists := ownerAnnotations["UserInfo"]; exists {
503+
var userInfo authv1beta1.UserInfo
504+
if err := json.Unmarshal([]byte(userInfoJSON), &userInfo); err != nil {
505+
return fmt.Errorf("unable to parse UserInfo annotation: %w", err)
506+
}
507+
508+
// Set user-related annotations using the parsed UserInfo struct
509+
r.Annotations[chaostypes.UserAnnotation] = userInfo.Username
510+
r.Annotations[chaostypes.UserGroupsAnnotation] = strings.Join(userInfo.Groups, ",")
511+
}
512+
513+
return nil
514+
}
515+
451516
// +kubebuilder:object:root=true
452517

453518
// DisruptionList contains a list of Disruption

api/v1beta1/disruption_types_test.go

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,4 +613,172 @@ var _ = Describe("Disruption", func() {
613613
Entry("with a disruption marked to be deleted not exceeding the timeout limit", builderstest.NewDisruptionBuilder().WithDeletion(), time.Minute*10, false),
614614
Entry("with a disruption marked to be deleted exceeding the timeout limit", builderstest.NewDisruptionBuilder().WithDeletion(), time.Minute*(-1), true),
615615
)
616+
617+
DescribeTable("CopyOwnerAnnotations", func(disruptionBuilder *builderstest.DisruptionBuilder, ownerAnnotations map[string]string, expectedAnnotations map[string]string) {
618+
// Arrange
619+
disruption := disruptionBuilder.Build()
620+
owner := &metav1.ObjectMeta{
621+
Annotations: ownerAnnotations,
622+
}
623+
624+
// Act
625+
disruption.CopyOwnerAnnotations(owner)
626+
627+
// Assert
628+
Expect(disruption.Annotations).NotTo(BeNil())
629+
Expect(disruption.Annotations).To(Equal(expectedAnnotations))
630+
},
631+
Entry("should copy annotations from the owner to the disruption",
632+
builderstest.NewDisruptionBuilder().WithAnnotations(map[string]string{
633+
"key3": "value3",
634+
}),
635+
map[string]string{"key1": "value1", "key2": "value2"},
636+
map[string]string{"key1": "value1", "key2": "value2", "key3": "value3"}),
637+
Entry("should create a new annotations map if it's nil",
638+
builderstest.NewDisruptionBuilder(),
639+
map[string]string{"key1": "value1", "key2": "value2"},
640+
map[string]string{"key1": "value1", "key2": "value2"}),
641+
)
642+
643+
Describe("SetScheduledAtAnnotation", func() {
644+
var scheduledTime time.Time
645+
646+
BeforeEach(func() {
647+
scheduledTime = time.Now()
648+
})
649+
650+
It("sets the scheduled time annotation", func() {
651+
// Arrange
652+
disruption := builderstest.NewDisruptionBuilder().Build()
653+
654+
// Act
655+
disruption.SetScheduledAtAnnotation(scheduledTime)
656+
657+
// Assert
658+
Expect(disruption.Annotations).To(HaveKey(chaostypes.ScheduledAtAnnotation))
659+
660+
parsedTime, err := time.Parse(time.RFC3339, disruption.Annotations[chaostypes.ScheduledAtAnnotation])
661+
Expect(err).NotTo(HaveOccurred())
662+
Expect(parsedTime).To(BeTemporally("~", scheduledTime, time.Second))
663+
})
664+
665+
It("creates a new annotations map if it's nil", func() {
666+
// Arrange
667+
disruption := builderstest.NewDisruptionBuilder().Build()
668+
669+
// Act
670+
disruption.SetScheduledAtAnnotation(scheduledTime)
671+
672+
// Act
673+
Expect(disruption.Annotations).NotTo(BeNil())
674+
Expect(disruption.Annotations).To(HaveKey(chaostypes.ScheduledAtAnnotation))
675+
})
676+
})
677+
678+
Describe("GetScheduledAtAnnotation", func() {
679+
Describe("success cases", func() {
680+
It("retrieves the scheduled time annotation", func() {
681+
// Arrange
682+
scheduledTime := time.Now().Format(time.RFC3339)
683+
disruption := builderstest.NewDisruptionBuilder().WithAnnotations(map[string]string{
684+
chaostypes.ScheduledAtAnnotation: scheduledTime,
685+
}).Build()
686+
687+
// Act
688+
retrievedTime, err := disruption.GetScheduledAtAnnotation()
689+
690+
// Assert
691+
Expect(err).NotTo(HaveOccurred())
692+
Expect(retrievedTime.Format(time.RFC3339)).To(Equal(scheduledTime))
693+
})
694+
})
695+
696+
Describe("error cases", func() {
697+
It("returns and error when the annotation is missing", func() {
698+
// Arrange
699+
disruption := builderstest.NewDisruptionBuilder().Build()
700+
701+
// Act
702+
_, err := disruption.GetScheduledAtAnnotation()
703+
704+
// Assert
705+
Expect(err).To(MatchError("scheduledAt annotation not found"))
706+
})
707+
708+
It("return and error when the annotation cannot be parsed", func() {
709+
// Arrange
710+
disruption := builderstest.NewDisruptionBuilder().WithAnnotations(map[string]string{
711+
chaostypes.ScheduledAtAnnotation: "invalid-time",
712+
}).Build()
713+
// Act
714+
_, err := disruption.GetScheduledAtAnnotation()
715+
716+
// Assert
717+
Expect(err).To(HaveOccurred())
718+
Expect(err.Error()).To(ContainSubstring("unable to parse scheduledAt annotation"))
719+
})
720+
})
721+
})
722+
723+
Describe("CopyUserInfoToAnnotations", func() {
724+
var owner *metav1.ObjectMeta
725+
726+
BeforeEach(func() {
727+
owner = &metav1.ObjectMeta{
728+
Annotations: map[string]string{
729+
"UserInfo": `{
730+
"username": "test-user",
731+
"groups": ["group1", "group2"]
732+
}`,
733+
"key1": "value1",
734+
"key2": "value2",
735+
},
736+
}
737+
})
738+
739+
Describe("success cases", func() {
740+
It("copies user-related annotations from the owner", func() {
741+
// Arrange
742+
disruption := builderstest.NewDisruptionBuilder().Build()
743+
744+
// Act
745+
err := disruption.CopyUserInfoToAnnotations(owner)
746+
747+
// Assert
748+
Expect(err).NotTo(HaveOccurred())
749+
Expect(disruption.Annotations).To(HaveKeyWithValue(chaostypes.UserAnnotation, "test-user"))
750+
Expect(disruption.Annotations).To(HaveKeyWithValue(chaostypes.UserGroupsAnnotation, "group1,group2"))
751+
})
752+
})
753+
754+
Describe("error cases", func() {
755+
It("returns an error if the UserInfo annotation is invalid JSON", func() {
756+
// Arrange
757+
disruption := builderstest.NewDisruptionBuilder().Build()
758+
owner.SetAnnotations(map[string]string{
759+
"UserInfo": "invalid-json",
760+
})
761+
762+
// Act
763+
err := disruption.CopyUserInfoToAnnotations(owner)
764+
765+
// Assert
766+
Expect(err).To(HaveOccurred())
767+
Expect(err.Error()).To(ContainSubstring("unable to parse UserInfo annotation"))
768+
})
769+
770+
It("does nothing if the UserInfo annotation does not exist", func() {
771+
// Arrange
772+
disruption := builderstest.NewDisruptionBuilder().Build()
773+
owner.SetAnnotations(map[string]string{})
774+
775+
// Act
776+
err := disruption.CopyUserInfoToAnnotations(owner)
777+
778+
// Assert
779+
Expect(err).NotTo(HaveOccurred())
780+
Expect(disruption.Annotations).To(BeEmpty())
781+
})
782+
})
783+
})
616784
})

builderstest/disruption.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,3 +234,20 @@ func (b *DisruptionBuilder) WithInjectionStatus(status types.DisruptionInjection
234234

235235
return b
236236
}
237+
238+
// WithAnnotations sets the specified annotations.
239+
func (b *DisruptionBuilder) WithAnnotations(annotations map[string]string) *DisruptionBuilder {
240+
b.modifiers = append(
241+
b.modifiers,
242+
func() {
243+
if b.ObjectMeta.Annotations == nil {
244+
b.ObjectMeta.Annotations = make(map[string]string)
245+
}
246+
247+
for k, v := range annotations {
248+
b.ObjectMeta.Annotations[k] = v
249+
}
250+
})
251+
252+
return b
253+
}

controllers/cron_rollout_helpers.go

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424
)
2525

2626
const (
27-
ScheduledAtAnnotation = chaosv1beta1.GroupName + "/scheduled-at"
2827
DisruptionCronNameLabel = chaosv1beta1.GroupName + "/disruption-cron-name"
2928
DisruptionRolloutNameLabel = chaosv1beta1.GroupName + "/disruption-rollout-name"
3029
TargetResourceMissingThreshold = time.Hour * 24
@@ -120,13 +119,14 @@ func createBaseDisruption(owner metav1.Object, disruptionSpec *chaosv1beta1.Disr
120119
}
121120

122121
// setDisruptionAnnotations updates the annotations of a given Disruption object with those of its owner.
123-
// Additionally, it sets a scheduled time annotation using the provided scheduledTime.
124-
func setDisruptionAnnotations(disruption *chaosv1beta1.Disruption, owner metav1.Object, scheduledTime time.Time) {
125-
for k, v := range owner.GetAnnotations() {
126-
disruption.Annotations[k] = v
127-
}
122+
// It sets a scheduled time annotation using the provided scheduledTime.
123+
// It parses the UserInfo annotation if it exists and sets user-related annotations.
124+
func setDisruptionAnnotations(disruption *chaosv1beta1.Disruption, owner metav1.Object, scheduledTime time.Time) error {
125+
disruption.CopyOwnerAnnotations(owner)
126+
127+
disruption.SetScheduledAtAnnotation(scheduledTime)
128128

129-
disruption.Annotations[ScheduledAtAnnotation] = scheduledTime.Format(time.RFC3339)
129+
return disruption.CopyUserInfoToAnnotations(owner)
130130
}
131131

132132
// overwriteDisruptionSelectors updates the selectors of a given Disruption object based on the provided targetResource.
@@ -156,13 +156,15 @@ func overwriteDisruptionSelectors(ctx context.Context, cl client.Client, disrupt
156156
// CreateDisruptionFromTemplate constructs a Disruption object based on the provided owner, disruptionSpec, and targetResource.
157157
// The function sets annotations, overwrites selectors, and associates the Disruption with its owner.
158158
// It returns the constructed Disruption or an error if any step fails.
159-
func CreateDisruptionFromTemplate(ctx context.Context, cl client.Client, scheme *runtime.Scheme, owner metav1.Object, targetResource *chaosv1beta1.TargetResourceSpec, disruptionSpec *chaosv1beta1.DisruptionSpec, scheduledTime time.Time) (*chaosv1beta1.Disruption, error) {
159+
func CreateDisruptionFromTemplate(ctx context.Context, cl client.Client, scheme *runtime.Scheme, owner metav1.Object, targetResource *chaosv1beta1.TargetResourceSpec, disruptionSpec *chaosv1beta1.DisruptionSpec, scheduledTime time.Time, log *zap.SugaredLogger) (*chaosv1beta1.Disruption, error) {
160160
disruption := createBaseDisruption(owner, disruptionSpec)
161161

162162
ownerNameLabel := getOwnerNameLabel(owner)
163163
disruption.Labels[ownerNameLabel] = owner.GetName()
164164

165-
setDisruptionAnnotations(disruption, owner, scheduledTime)
165+
if err := setDisruptionAnnotations(disruption, owner, scheduledTime); err != nil {
166+
log.Errorw("unable to set annotations for child disruption", "err", err, "disruptionName", disruption.Name)
167+
}
166168

167169
if err := overwriteDisruptionSelectors(ctx, cl, disruption, targetResource, owner.GetNamespace()); err != nil {
168170
return nil, err
@@ -177,18 +179,13 @@ func CreateDisruptionFromTemplate(ctx context.Context, cl client.Client, scheme
177179

178180
// getScheduledTimeForDisruption returns the scheduled time for a particular disruption.
179181
func getScheduledTimeForDisruption(log *zap.SugaredLogger, disruption *chaosv1beta1.Disruption) time.Time {
180-
timeRaw := disruption.Annotations[ScheduledAtAnnotation]
181-
if len(timeRaw) == 0 {
182-
return time.Time{}
183-
}
184-
185-
timeParsed, err := time.Parse(time.RFC3339, timeRaw)
182+
parsedTime, err := disruption.GetScheduledAtAnnotation()
186183
if err != nil {
187184
log.Errorw("unable to parse schedule time for child disruption", "err", err, "disruptionName", disruption.Name)
188185
return time.Time{}
189186
}
190187

191-
return timeParsed
188+
return parsedTime
192189
}
193190

194191
// GetMostRecentScheduleTime returns the most recent scheduled time from a list of disruptions.

controllers/disruption_cron_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ func (r *DisruptionCronReconciler) Reconcile(ctx context.Context, req ctrl.Reque
170170
r.log.Infow("processing current run", "currentRun", missedRun.Format(time.UnixDate))
171171

172172
// Create disruption for current run
173-
disruption, err := CreateDisruptionFromTemplate(ctx, r.Client, r.Scheme, instance, &instance.Spec.TargetResource, &instance.Spec.DisruptionTemplate, missedRun)
173+
disruption, err := CreateDisruptionFromTemplate(ctx, r.Client, r.Scheme, instance, &instance.Spec.TargetResource, &instance.Spec.DisruptionTemplate, missedRun, r.log)
174174

175175
if err != nil {
176176
r.log.Warnw("unable to construct disruption from template", "err", err)

controllers/disruption_rollout_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func (r *DisruptionRolloutReconciler) Reconcile(ctx context.Context, req ctrl.Re
136136

137137
// Create disruption
138138
scheduledTime := time.Now()
139-
disruption, err := CreateDisruptionFromTemplate(ctx, r.Client, r.Scheme, instance, &instance.Spec.TargetResource, &instance.Spec.DisruptionTemplate, scheduledTime)
139+
disruption, err := CreateDisruptionFromTemplate(ctx, r.Client, r.Scheme, instance, &instance.Spec.TargetResource, &instance.Spec.DisruptionTemplate, scheduledTime, r.log)
140140

141141
if err != nil {
142142
r.log.Warnw("unable to construct disruption from template", "err", err)

types/types.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,14 @@ const (
120120
// DisruptionNamespaceLabel is the label used to identify the disruption namespace for a chaos pod. This is used to determine pod ownership.
121121
DisruptionNamespaceLabel = GroupName + "/disruption-namespace"
122122

123+
// ScheduledAtAnnotation is the annotation key for the scheduled time of the disruption when it is created from DisruptionCron or DisruptionRollout.
124+
ScheduledAtAnnotation = GroupName + "/scheduled-at"
125+
126+
// UserAnnotation is the annotation key that stores the username of the user who created the parent resource (DisruptionCron or DisruptionRollout).
127+
UserAnnotation = GroupName + "/user"
128+
// UserGroupsAnnotation is the annotation key that stores the user groups of the individual who created the parent resource (DisruptionCron or DisruptionRollout).
129+
UserGroupsAnnotation = GroupName + "/user-groups"
130+
123131
finalizerPrefix = "finalizer." + GroupName
124132
DisruptionFinalizer = finalizerPrefix
125133
DisruptionCronFinalizer = finalizerPrefix

0 commit comments

Comments
 (0)