Skip to content
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

pkg/webhook: refactor Federated Resource Quota test #5544

Merged
merged 1 commit into from
Sep 21, 2024
Merged

pkg/webhook: refactor Federated Resource Quota test #5544

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
145 changes: 102 additions & 43 deletions pkg/webhook/federatedresourcequota/validating_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,28 @@ import (
policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
)

type fakeDecoder struct {
// ResponseType represents the type of admission response.
type ResponseType string

const (
Denied ResponseType = "Denied"
Allowed ResponseType = "Allowed"
Errored ResponseType = "Errored"
)

// TestResponse is used to define expected response in a test case.
type TestResponse struct {
Type ResponseType
Message string
}

type fakeValidationDecoder struct {
err error
obj runtime.Object
}

// Decode mocks the Decode method of admission.Decoder.
func (f *fakeDecoder) Decode(_ admission.Request, obj runtime.Object) error {
func (f *fakeValidationDecoder) Decode(_ admission.Request, obj runtime.Object) error {
if f.err != nil {
return f.err
}
Expand All @@ -53,7 +68,7 @@ func (f *fakeDecoder) Decode(_ admission.Request, obj runtime.Object) error {
}

// DecodeRaw mocks the DecodeRaw method of admission.Decoder.
func (f *fakeDecoder) DecodeRaw(_ runtime.RawExtension, obj runtime.Object) error {
func (f *fakeValidationDecoder) DecodeRaw(_ runtime.RawExtension, obj runtime.Object) error {
if f.err != nil {
return f.err
}
Expand All @@ -66,7 +81,7 @@ func (f *fakeDecoder) DecodeRaw(_ runtime.RawExtension, obj runtime.Object) erro
// sortAndJoinMessages sorts and joins error message parts to ensure consistent ordering.
// This prevents test flakiness caused by varying error message order.
func sortAndJoinMessages(message string) string {
parts := strings.Split(strings.Trim(message, "[]"), ", ")
parts := strings.Split(message, ", ")
sort.Strings(parts)
return strings.Join(parts, ", ")
}
Expand All @@ -86,7 +101,7 @@ func Test_validateOverallAndAssignments(t *testing.T) {
want field.ErrorList
}{
{
"normal",
"validateOverallAndAssignments_ValidValues_NoErrors",
args{
quotaSpec: &policyv1alpha1.FederatedResourceQuotaSpec{
Overall: corev1.ResourceList{
Expand Down Expand Up @@ -115,7 +130,7 @@ func Test_validateOverallAndAssignments(t *testing.T) {
field.ErrorList{},
},
{
"overall[cpu] is less than assignments",
"validateOverallAndAssignments_LowerOverallCPUThanAssignments_OverallCPULessThanAssignments",
args{
quotaSpec: &policyv1alpha1.FederatedResourceQuotaSpec{
Overall: corev1.ResourceList{
Expand Down Expand Up @@ -146,7 +161,7 @@ func Test_validateOverallAndAssignments(t *testing.T) {
},
},
{
"overall[memory] is less than assignments",
"validateOverallAndAssignments_LowerOverallMemoryThanAssignments_OverallMemoryLessThanAssignments",
args{
quotaSpec: &policyv1alpha1.FederatedResourceQuotaSpec{
Overall: corev1.ResourceList{
Expand Down Expand Up @@ -177,7 +192,7 @@ func Test_validateOverallAndAssignments(t *testing.T) {
},
},
{
"assignment resourceName is not exist in overall",
"validateOverallAndAssignments_InvalidAssignmentResource_AssignmentResourceNotInOverall",
args{
quotaSpec: &policyv1alpha1.FederatedResourceQuotaSpec{
Overall: corev1.ResourceList{
Expand Down Expand Up @@ -222,19 +237,22 @@ func TestValidatingAdmission_Handle(t *testing.T) {
name string
decoder admission.Decoder
req admission.Request
want admission.Response
want TestResponse
}{
{
name: "Decode Error Handling",
decoder: &fakeDecoder{
name: "Handle_DecodeErrorHandling_ErrorReturned",
decoder: &fakeValidationDecoder{
err: errors.New("decode error"),
},
req: admission.Request{},
want: admission.Errored(http.StatusBadRequest, errors.New("decode error")),
req: admission.Request{},
want: TestResponse{
Type: Errored,
Message: "decode error",
},
},
{
name: "Validation Success - Resource Limits Match",
decoder: &fakeDecoder{
name: "Handle_ResourceLimitsMatch_NoErrors",
decoder: &fakeValidationDecoder{
obj: &policyv1alpha1.FederatedResourceQuota{
Spec: policyv1alpha1.FederatedResourceQuotaSpec{
Overall: corev1.ResourceList{
Expand All @@ -260,12 +278,15 @@ func TestValidatingAdmission_Handle(t *testing.T) {
},
},
},
req: admission.Request{},
want: admission.Allowed(""),
req: admission.Request{},
want: TestResponse{
Type: Allowed,
Message: "",
},
},
{
name: "Validation Error - Resource Limits Exceeded",
decoder: &fakeDecoder{
name: "Handle_ExceedResourceLimits_ResourceLimitsExceeded",
decoder: &fakeValidationDecoder{
obj: &policyv1alpha1.FederatedResourceQuota{
Spec: policyv1alpha1.FederatedResourceQuotaSpec{
Overall: corev1.ResourceList{
Expand All @@ -291,12 +312,15 @@ func TestValidatingAdmission_Handle(t *testing.T) {
},
},
},
req: admission.Request{},
want: admission.Denied(fmt.Sprintf("[spec.overall[cpu]: Invalid value: \"%s\": overall is less than assignments, spec.overall[memory]: Invalid value: \"%s\": overall is less than assignments]", "5", "5Gi")),
req: admission.Request{},
want: TestResponse{
Type: Denied,
Message: fmt.Sprintf("spec.overall[cpu]: Invalid value: \"%s\": overall is less than assignments, spec.overall[memory]: Invalid value: \"%s\": overall is less than assignments", "5", "5Gi"),
},
},
{
name: "Validation Error - CPU Allocation Exceeds Overall Limit",
decoder: &fakeDecoder{
name: "Handle_ExceedCPUAllocationOverallLimit_CPUAllocationExceedsOverallLimit",
decoder: &fakeValidationDecoder{
obj: &policyv1alpha1.FederatedResourceQuota{
Spec: policyv1alpha1.FederatedResourceQuotaSpec{
Overall: corev1.ResourceList{
Expand All @@ -322,12 +346,15 @@ func TestValidatingAdmission_Handle(t *testing.T) {
},
},
},
req: admission.Request{},
want: admission.Denied(fmt.Sprintf("spec.overall[cpu]: Invalid value: \"%s\": overall is less than assignments", "5")),
req: admission.Request{},
want: TestResponse{
Type: Denied,
Message: fmt.Sprintf("spec.overall[cpu]: Invalid value: \"%s\": overall is less than assignments", "5"),
},
},
{
name: "Validation Error - Memory Allocation Exceeds Overall Limit",
decoder: &fakeDecoder{
name: "Handle_ExceedMemoryAllocationOverallLimit_MemoryAllocationExceedsOverallLimit",
decoder: &fakeValidationDecoder{
obj: &policyv1alpha1.FederatedResourceQuota{
Spec: policyv1alpha1.FederatedResourceQuotaSpec{
Overall: corev1.ResourceList{
Expand All @@ -353,12 +380,15 @@ func TestValidatingAdmission_Handle(t *testing.T) {
},
},
},
req: admission.Request{},
want: admission.Denied(fmt.Sprintf("spec.overall[memory]: Invalid value: \"%s\": overall is less than assignments", "5Gi")),
req: admission.Request{},
want: TestResponse{
Type: Denied,
Message: fmt.Sprintf("spec.overall[memory]: Invalid value: \"%s\": overall is less than assignments", "5Gi"),
},
},
{
name: "Invalid Cluster Name",
decoder: &fakeDecoder{
name: "Handle_InvalidClusterName_ClusterNameInvalid",
decoder: &fakeValidationDecoder{
obj: &policyv1alpha1.FederatedResourceQuota{
Spec: policyv1alpha1.FederatedResourceQuotaSpec{
Overall: corev1.ResourceList{
Expand All @@ -377,8 +407,11 @@ func TestValidatingAdmission_Handle(t *testing.T) {
},
},
},
req: admission.Request{},
want: admission.Denied("[spec.staticAssignments[0].clusterName: Invalid value: \"invalid cluster name\": a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')]"),
req: admission.Request{},
want: TestResponse{
Type: Denied,
Message: "Invalid value: \"invalid cluster name\"",
},
},
}

Expand All @@ -389,9 +422,14 @@ func TestValidatingAdmission_Handle(t *testing.T) {
}
got := v.Handle(context.Background(), tt.req)
got.Result.Message = sortAndJoinMessages(got.Result.Message)
tt.want.Result.Message = sortAndJoinMessages(tt.want.Result.Message)
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("Handle() = %v, want %v", got, tt.want)
tt.want.Message = sortAndJoinMessages(tt.want.Message)

// Extract type and message from the actual response.
gotType := extractResponseType(got)
gotMessage := extractErrorMessage(got)

if gotType != tt.want.Type || !strings.Contains(gotMessage, tt.want.Message) {
t.Errorf("Handle() = {Type: %v, Message: %v}, want {Type: %v, Message: %v}", gotType, gotMessage, tt.want.Type, tt.want.Message)
}
})
}
Expand All @@ -406,7 +444,7 @@ func Test_validateFederatedResourceQuotaStatus(t *testing.T) {
expected field.ErrorList
}{
{
name: "Valid FederatedResourceQuotaStatus",
name: "validateFederatedResourceQuotaStatus_ValidStatus_NoErrors",
status: &policyv1alpha1.FederatedResourceQuotaStatus{
Overall: corev1.ResourceList{"cpu": resource.MustParse("10")},
OverallUsed: corev1.ResourceList{"cpu": resource.MustParse("5")},
Expand All @@ -423,7 +461,7 @@ func Test_validateFederatedResourceQuotaStatus(t *testing.T) {
expected: field.ErrorList{},
},
{
name: "Invalid Overall Resource List",
name: "validateFederatedResourceQuotaStatus_InvalidOverallResourceList_ResourceTypeInvalid",
status: &policyv1alpha1.FederatedResourceQuotaStatus{
Overall: corev1.ResourceList{"invalid-resource": resource.MustParse("10")},
OverallUsed: corev1.ResourceList{"cpu": resource.MustParse("5")},
Expand All @@ -443,7 +481,7 @@ func Test_validateFederatedResourceQuotaStatus(t *testing.T) {
},
},
{
name: "Invalid AggregatedStatus Resource List",
name: "validateFederatedResourceQuotaStatus_InvalidAggregatedStatusResourceList_ResourceTypeInvalid",
status: &policyv1alpha1.FederatedResourceQuotaStatus{
Overall: corev1.ResourceList{"cpu": resource.MustParse("10")},
OverallUsed: corev1.ResourceList{"cpu": resource.MustParse("5")},
Expand Down Expand Up @@ -482,7 +520,7 @@ func Test_validateClusterQuotaStatus(t *testing.T) {
expected field.ErrorList
}{
{
name: "Valid ClusterQuotaStatus",
name: "validateClusterQuotaStatus_ValidClusterQuotaStatus_NoErrors",
status: &policyv1alpha1.ClusterQuotaStatus{
ClusterName: "valid-cluster",
ResourceQuotaStatus: corev1.ResourceQuotaStatus{
Expand All @@ -493,7 +531,7 @@ func Test_validateClusterQuotaStatus(t *testing.T) {
expected: field.ErrorList{},
},
{
name: "Invalid Cluster Name",
name: "validateClusterQuotaStatus_InvalidClusterName_ClusterNameInvalid",
status: &policyv1alpha1.ClusterQuotaStatus{
ClusterName: "invalid cluster name",
ResourceQuotaStatus: corev1.ResourceQuotaStatus{
Expand All @@ -506,7 +544,7 @@ func Test_validateClusterQuotaStatus(t *testing.T) {
},
},
{
name: "Invalid Resource List - Hard",
name: "validateClusterQuotaStatus_InvalidResourceList_HardResourceInvalid",
status: &policyv1alpha1.ClusterQuotaStatus{
ClusterName: "valid-cluster",
ResourceQuotaStatus: corev1.ResourceQuotaStatus{
Expand All @@ -520,7 +558,7 @@ func Test_validateClusterQuotaStatus(t *testing.T) {
},
},
{
name: "Invalid Resource List - Used",
name: "validateClusterQuotaStatus_InvalidResourceList_UsedResourceInvalid",
status: &policyv1alpha1.ClusterQuotaStatus{
ClusterName: "valid-cluster",
ResourceQuotaStatus: corev1.ResourceQuotaStatus{
Expand All @@ -543,3 +581,24 @@ func Test_validateClusterQuotaStatus(t *testing.T) {
})
}
}

// extractResponseType extracts the type of admission response.
func extractResponseType(resp admission.Response) ResponseType {
if resp.Allowed {
return Allowed
}
if resp.Result != nil {
if resp.Result.Code == http.StatusBadRequest {
return Errored
}
}
return Denied
}

// extractErrorMessage extracts the error message from a Denied/Errored response.
func extractErrorMessage(resp admission.Response) string {
if !resp.Allowed && resp.Result != nil {
return resp.Result.Message
}
return ""
}