Skip to content

feat: add validation for timeoutInSeconds for MonitorConfig in TrafficManagerProfile #292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zhiying-lin merged 3 commits into from
Apr 23, 2025
Merged

feat: add validation for timeoutInSeconds for MonitorConfig in TrafficManagerProfile #292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a0606c5
feat: add validation for timeoutInSeconds and tests
britaniar Apr 17, 2025
b662d8e
address comments
britaniar Apr 18, 2025
877a71a
remove duplicate check
britaniar Apr 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/v1beta1/trafficmanagerprofile_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ type TrafficManagerProfileSpec struct {

// MonitorConfig defines the endpoint monitoring settings of the Traffic Manager profile.
// https://learn.microsoft.com/en-us/azure/traffic-manager/traffic-manager-monitoring
// +kubebuilder:validation:XValidation:rule="has(self.intervalInSeconds) && self.intervalInSeconds == 30 ? (!has(self.timeoutInSeconds) || (self.timeoutInSeconds >= 5 && self.timeoutInSeconds <= 10)) : true",message="timeoutInSeconds must be between 5 and 10 when intervalInSeconds is 30"
// +kubebuilder:validation:XValidation:rule="has(self.intervalInSeconds) && self.intervalInSeconds == 10 ? (!has(self.timeoutInSeconds) || (self.timeoutInSeconds >= 5 && self.timeoutInSeconds <= 9)) : true",message="timeoutInSeconds must be between 5 and 9 when intervalInSeconds is 10"
type MonitorConfig struct {
// The monitor interval for endpoints in this profile. This is the interval at which Traffic Manager will check the health
// of each endpoint in this profile.
Expand Down
11 changes: 11 additions & 0 deletions config/crd/bases/networking.fleet.azure.com_trafficmanagerprofiles.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,17 @@ spec:
minimum: 0
type: integer
type: object
x-kubernetes-validations:
- message: timeoutInSeconds must be between 5 and 10 when intervalInSeconds
is 30
rule: 'has(self.intervalInSeconds) && self.intervalInSeconds ==
30 ? (!has(self.timeoutInSeconds) || (self.timeoutInSeconds >=
5 && self.timeoutInSeconds <= 10)) : true'
- message: timeoutInSeconds must be between 5 and 9 when intervalInSeconds
is 10
rule: 'has(self.intervalInSeconds) && self.intervalInSeconds ==
10 ? (!has(self.timeoutInSeconds) || (self.timeoutInSeconds >=
5 && self.timeoutInSeconds <= 9)) : true'
resourceGroup:
description: |-
The name of the resource group to contain the Azure Traffic Manager resource corresponding to this profile.
Expand Down
18 changes: 12 additions & 6 deletions pkg/controllers/hub/trafficmanagerprofile/controller_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,18 @@ var _ = Describe("Test TrafficManagerProfile Controller", func() {
validateTrafficManagerProfileMetricsEmitted(customRegistry, wantMetrics...)
})

It("Update the trafficManagerProfile spec", func() {
It("Update the trafficManagerProfile spec and should fail", func() {
Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: testNamespace, Name: name}, profile)).Should(Succeed(), "failed to get the trafficManagerProfile")
profile.Spec.MonitorConfig.IntervalInSeconds = ptr.To[int64](10)
profile.Spec.MonitorConfig.TimeoutInSeconds = ptr.To[int64](10)
Expect(k8sClient.Update(ctx, profile)).Should(Succeed(), "failed to update the trafficManagerProfile")
Expect(k8sClient.Update(ctx, profile)).ShouldNot(Succeed(), "failed to update the trafficManagerProfile")
})

It("Validating trafficManagerProfile status and update should fail", func() {
It("Updating trafficManagerProfile spec to valid and validating trafficManagerProfile status", func() {
profile.Spec.MonitorConfig.IntervalInSeconds = ptr.To[int64](30)
profile.Spec.MonitorConfig.TimeoutInSeconds = ptr.To[int64](10)
Expect(k8sClient.Update(ctx, profile)).Should(Succeed(), "failed to update the trafficManagerProfile")

want := fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Expand All @@ -170,11 +174,13 @@ var _ = Describe("Test TrafficManagerProfile Controller", func() {
},
Spec: profile.Spec,
Status: fleetnetv1beta1.TrafficManagerProfileStatus{
DNSName: ptr.To(fqdn),
ResourceID: profileResourceID,
Conditions: []metav1.Condition{
{
Status: metav1.ConditionFalse,
Status: metav1.ConditionTrue,
Type: string(fleetnetv1beta1.TrafficManagerProfileConditionProgrammed),
Reason: string(fleetnetv1beta1.TrafficManagerProfileReasonInvalid),
Reason: string(fleetnetv1beta1.TrafficManagerProfileReasonProgrammed),
ObservedGeneration: profile.Generation,
},
},
Expand All @@ -183,7 +189,7 @@ var _ = Describe("Test TrafficManagerProfile Controller", func() {
validator.ValidateTrafficManagerProfile(ctx, k8sClient, &want, timeout)

By("By validating the status metrics")
wantMetrics = append(wantMetrics, generateMetrics(profile, want.Status.Conditions[0]))
// It overwrites the previous one as they have the same condition.
validateTrafficManagerProfileMetricsEmitted(customRegistry, wantMetrics...)
})

Expand Down
236 changes: 236 additions & 0 deletions test/apis/v1beta1/api_validation_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,110 @@ var _ = Describe("Test networking v1alpha1 API validation", func() {
Expect(hubClient.Delete(ctx, profile)).Should(Succeed(), "failed to delete trafficManagerProfile")
})

It("should deny creating API with timeoutInSeconds < 5 when intervalInSeconds is 30", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
IntervalInSeconds: ptr.To(int64(30)),
TimeoutInSeconds: ptr.To(int64(4)),
},
ResourceGroup: "test-resource-group",
},
}
By("expecting denial of CREATE API with invalid timeoutInSeconds")
var err = hubClient.Create(ctx, profile)
Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create API call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8serrors.StatusError{})))
Expect(statusErr.Status().Message).Should(ContainSubstring("spec.monitorConfig.timeoutInSeconds in body should be greater than or equal to 5"))
Expect(statusErr.Status().Message).Should(ContainSubstring("timeoutInSeconds must be between 5 and 10 when intervalInSeconds is 30"))
})

It("should deny creating API with timeoutInSeconds > 10 when intervalInSeconds is 30", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
IntervalInSeconds: ptr.To(int64(30)),
TimeoutInSeconds: ptr.To(int64(15)),
},
ResourceGroup: "test-resource-group",
},
}
By("expecting denial of CREATE API with invalid timeoutInSeconds")
var err = hubClient.Create(ctx, profile)
Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create API call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8serrors.StatusError{})))
Expect(statusErr.Status().Message).Should(ContainSubstring("spec.monitorConfig.timeoutInSeconds in body should be less than or equal to 10"))
Expect(statusErr.Status().Message).Should(ContainSubstring("timeoutInSeconds must be between 5 and 10 when intervalInSeconds is 30"))
})

It("should deny creating API with timeoutInSeconds < 5 when intervalInSeconds is 10", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
IntervalInSeconds: ptr.To(int64(10)),
TimeoutInSeconds: ptr.To(int64(4)),
},
ResourceGroup: "test-resource-group",
},
}
By("expecting denial of CREATE API with invalid timeoutInSeconds")
var err = hubClient.Create(ctx, profile)
Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create API call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8serrors.StatusError{})))
Expect(statusErr.Status().Message).Should(ContainSubstring("spec.monitorConfig.timeoutInSeconds in body should be greater than or equal to 5"))
Expect(statusErr.Status().Message).Should(ContainSubstring("timeoutInSeconds must be between 5 and 9 when intervalInSeconds is 10"))
})

It("should deny creating API with timeoutInSeconds > 9 when intervalInSeconds is 10", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
IntervalInSeconds: ptr.To(int64(10)),
TimeoutInSeconds: ptr.To(int64(10)),
},
ResourceGroup: "test-resource-group",
},
}
By("expecting denial of CREATE API with invalid timeoutInSeconds")
var err = hubClient.Create(ctx, profile)
Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create API call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8serrors.StatusError{})))
Expect(statusErr.Status().Message).Should(ContainSubstring("spec.monitorConfig: Invalid value: \"object\": timeoutInSeconds must be between 5 and 9 when intervalInSeconds is 10"))
})

It("should deny creating API with timeoutInSeconds < 5 when intervalInSeconds is not defined", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
TimeoutInSeconds: ptr.To(int64(3)),
},
ResourceGroup: "test-resource-group",
},
}
By("expecting denial of CREATE API with invalid timeoutInSeconds")
var err = hubClient.Create(ctx, profile)
Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create API call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8serrors.StatusError{})))
Expect(statusErr.Status().Message).Should(ContainSubstring("spec.monitorConfig.timeoutInSeconds in body should be greater than or equal to 5"))
Expect(statusErr.Status().Message).Should(ContainSubstring("timeoutInSeconds must be between 5 and 10 when intervalInSeconds is 30"))
})

It("should deny creating API with timeoutInSeconds > 10 when intervalInSeconds is not defined", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
TimeoutInSeconds: ptr.To(int64(11)),
},
ResourceGroup: "test-resource-group",
},
}
By("expecting denial of CREATE API with invalid timeoutInSeconds")
var err = hubClient.Create(ctx, profile)
Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create API call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8serrors.StatusError{})))
Expect(statusErr.Status().Message).Should(ContainSubstring("spec.monitorConfig.timeoutInSeconds in body should be less than or equal to 10"))
Expect(statusErr.Status().Message).Should(ContainSubstring("timeoutInSeconds must be between 5 and 10 when intervalInSeconds is 30"))
})
})

Context("Test TrafficManagerProfile API validation - valid cases", func() {
Expand Down Expand Up @@ -231,6 +335,138 @@ var _ = Describe("Test networking v1alpha1 API validation", func() {
Expect(hubClient.Create(ctx, trafficManagerProfileName)).Should(Succeed(), "failed to create trafficManagerProfile")
Expect(hubClient.Delete(ctx, trafficManagerProfileName)).Should(Succeed(), "failed to delete trafficManagerProfile")
})

It("should allow creating API with valid timeoutInSeconds when intervalInSeconds is 30", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
IntervalInSeconds: ptr.To(int64(30)),
TimeoutInSeconds: ptr.To(int64(7)),
},
ResourceGroup: "test-resource-group",
},
}
Expect(hubClient.Create(ctx, profile)).Should(Succeed(), "failed to create trafficManagerProfile")
Expect(hubClient.Delete(ctx, profile)).Should(Succeed(), "failed to delete trafficManagerProfile")
})

It("should allow creating API with timeoutInSeconds at lower bound (5) when intervalInSeconds is 30", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
IntervalInSeconds: ptr.To(int64(30)),
TimeoutInSeconds: ptr.To(int64(5)),
},
ResourceGroup: "test-resource-group",
},
}
Expect(hubClient.Create(ctx, profile)).Should(Succeed(), "failed to create trafficManagerProfile at lower bound")
Expect(hubClient.Delete(ctx, profile)).Should(Succeed(), "failed to delete trafficManagerProfile at lower bound")
})

It("should allow creating API with timeoutInSeconds at upper bound (10) when intervalInSeconds is 30", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
IntervalInSeconds: ptr.To(int64(30)),
TimeoutInSeconds: ptr.To(int64(10)),
},
ResourceGroup: "test-resource-group",
},
}
Expect(hubClient.Create(ctx, profile)).Should(Succeed(), "failed to create trafficManagerProfile at upper bound")
Expect(hubClient.Delete(ctx, profile)).Should(Succeed(), "failed to delete trafficManagerProfile at upper bound")
})

It("should allow creating API with valid timeoutInSeconds when intervalInSeconds is 10", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
IntervalInSeconds: ptr.To(int64(10)),
TimeoutInSeconds: ptr.To(int64(8)),
},
ResourceGroup: "test-resource-group",
},
}
Expect(hubClient.Create(ctx, profile)).Should(Succeed(), "failed to create trafficManagerProfile")
Expect(hubClient.Delete(ctx, profile)).Should(Succeed(), "failed to delete trafficManagerProfile")
})

It("should allow creating API with timeoutInSeconds at lower bound (5) when intervalInSeconds is 10", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
IntervalInSeconds: ptr.To(int64(10)),
TimeoutInSeconds: ptr.To(int64(5)),
},
ResourceGroup: "test-resource-group",
},
}
Expect(hubClient.Create(ctx, profile)).Should(Succeed(), "failed to create trafficManagerProfile at lower bound")
Expect(hubClient.Delete(ctx, profile)).Should(Succeed(), "failed to delete trafficManagerProfile at lower bound")
})

It("should allow creating API with timeoutInSeconds at upper bound (9) when intervalInSeconds is 10", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
IntervalInSeconds: ptr.To(int64(30)),
TimeoutInSeconds: ptr.To(int64(9)),
},
ResourceGroup: "test-resource-group",
},
}
Expect(hubClient.Create(ctx, profile)).Should(Succeed(), "failed to create trafficManagerProfile at upper bound")
Expect(hubClient.Delete(ctx, profile)).Should(Succeed(), "failed to delete trafficManagerProfile at upper bound")
})

It("should allow creating API with valid timeoutInSeconds when intervalInSeconds is not defined", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
TimeoutInSeconds: ptr.To(int64(10)),
},
ResourceGroup: "test-resource-group",
},
}
Expect(hubClient.Create(ctx, profile)).Should(Succeed(), "failed to create trafficManagerProfile")
Expect(hubClient.Delete(ctx, profile)).Should(Succeed(), "failed to delete trafficManagerProfile")
})

It("should allow creating API with timeoutInSeconds at lower bound (5) when intervalInSeconds is not defined", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
TimeoutInSeconds: ptr.To(int64(5)),
},
ResourceGroup: "test-resource-group",
},
}
Expect(hubClient.Create(ctx, profile)).Should(Succeed(), "failed to create trafficManagerProfile at lower bound")
Expect(hubClient.Delete(ctx, profile)).Should(Succeed(), "failed to delete trafficManagerProfile at lower bound")
})

It("should allow creating API with timeoutInSeconds at upper bound (10) when intervalInSeconds is not defined", func() {
profile := &fleetnetv1beta1.TrafficManagerProfile{
ObjectMeta: objectMetaWithNameValid,
Spec: fleetnetv1beta1.TrafficManagerProfileSpec{
MonitorConfig: &fleetnetv1beta1.MonitorConfig{
TimeoutInSeconds: ptr.To(int64(10)),
},
ResourceGroup: "test-resource-group",
},
}
Expect(hubClient.Create(ctx, profile)).Should(Succeed(), "failed to create trafficManagerProfile at upper bound")
Expect(hubClient.Delete(ctx, profile)).Should(Succeed(), "failed to delete trafficManagerProfile at upper bound")
})
})

Context("Test TrafficManagerBackend API validation - invalid cases", func() {
Expand Down
Loading