Skip to content

Commit

Permalink
Added validation for spec.memStorage.size does not surpass spec.resou…
Browse files Browse the repository at this point in the history 
…rces
  • Loading branch information
@mfaizanse
mfaizanse committed Jun 21, 2023
1 parent fb8315e commit 2c0fc82
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 103 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@ bin/*

# Vendor
vendor/
.env.dev
1 change: 1 addition & 0 deletions api/v1alpha1/nats_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ type NATS struct {
metav1.ObjectMeta `json:"metadata,omitempty"`

// +kubebuilder:default:={jetStream:{fileStorage:{storageClassName:"default", size:"1Gi"},memStorage:{size:"20Mi",enabled:false}}, cluster:{size:3},logging:{trace:false,debug:false}, resources:{limits:{cpu:"20m",memory:"64Mi"}, requests:{cpu:"5m",memory:"16Mi"}}}
// +kubebuilder:validation:XValidation:rule="!self.jetStream.memStorage.enabled || self.jetStream.memStorage.size < self.resources.limits.memory", message="memStorage.size should be less than resources.limits.memory."
Spec NATSSpec `json:"spec,omitempty"`
Status NATSStatus `json:"status,omitempty"`
}
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions config/crd/bases/operator.kyma-project.io_nats.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,10 @@ spec:
type: object
type: object
type: object
x-kubernetes-validations:
- message: memStorage.size should be less than resources.limits.memory.
rule: '!self.jetStream.memStorage.enabled || self.jetStream.memStorage.size
< self.resources.limits.memory'
status:
description: NATSStatus defines the observed state of NATS.
properties:
Expand Down
193 changes: 91 additions & 102 deletions internal/controller/nats/integrationtests/validation/integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,133 +70,118 @@ func Test_Validate_CreateNATS(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
// We use Unstructured instead of NATS to ensure that all undefined properties are nil and not Go defaults.
givenUnstructuredNATS unstructured.Unstructured
wantErrMsg string
name string
givenNATS *v1alpha1.NATS
wantErrMsg string
}{
{
name: `validation of spec.cluster.size passes for odd numbers`,
givenUnstructuredNATS: unstructured.Unstructured{
Object: map[string]any{
kind: kindNATS,
apiVersion: apiVersionNATS,
metadata: map[string]any{
name: testutils.GetRandK8sName(7),
namespace: testutils.GetRandK8sName(7),
},
spec: map[string]any{
cluster: map[string]any{
size: 3,
},
},
},
},
givenNATS: testutils.NewNATSCR(
testutils.WithNATSClusterSize(3),
),
wantErrMsg: noError,
},
{
name: `validation of spec.cluster.size fails for even numbers`,
givenUnstructuredNATS: unstructured.Unstructured{
Object: map[string]any{
kind: kindNATS,
apiVersion: apiVersionNATS,
metadata: map[string]any{
name: testutils.GetRandK8sName(7),
namespace: testutils.GetRandK8sName(7),
},
spec: map[string]any{
cluster: map[string]any{
size: 4,
},
},
},
},
givenNATS: testutils.NewNATSCR(
testutils.WithNATSClusterSize(4),
),
wantErrMsg: "size only accepts odd numbers",
},
{
name: `validation of spec.cluster.size fails for numbers < 1`,
givenUnstructuredNATS: unstructured.Unstructured{
Object: map[string]any{
kind: kindNATS,
apiVersion: apiVersionNATS,
metadata: map[string]any{
name: testutils.GetRandK8sName(7),
namespace: testutils.GetRandK8sName(7),
},
spec: map[string]any{
cluster: map[string]any{
size: -1,
},
},
},
},
givenNATS: testutils.NewNATSCR(
testutils.WithNATSClusterSize(-1),
),
wantErrMsg: "should be greater than or equal to 1",
},
{
name: `validation of spec.jetStream.memStorage passes if enabled is true and size is not 0`,
givenUnstructuredNATS: unstructured.Unstructured{
Object: map[string]any{
kind: kindNATS,
apiVersion: apiVersionNATS,
metadata: map[string]any{
name: testutils.GetRandK8sName(7),
namespace: testutils.GetRandK8sName(7),
},
spec: map[string]any{
jetStream: map[string]any{
memStorage: map[string]any{
enabled: true,
size: "1Gi",
},
},
givenNATS: testutils.NewNATSCR(
testutils.WithNATSMemStorage(v1alpha1.MemStorage{
Enabled: true,
Size: resource.MustParse("1Gi"),
}),
testutils.WithNATSResources(corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"memory": resource.MustParse("2Gi"),
},
},
},
}),
),
wantErrMsg: noError,
},
{
name: `validation of spec.jetStream.memStorage passes if size is 0 but enabled is false`,
givenUnstructuredNATS: unstructured.Unstructured{
Object: map[string]any{
kind: kindNATS,
apiVersion: apiVersionNATS,
metadata: map[string]any{
name: testutils.GetRandK8sName(7),
namespace: testutils.GetRandK8sName(7),
givenNATS: testutils.NewNATSCR(
testutils.WithNATSMemStorage(v1alpha1.MemStorage{
Enabled: false,
Size: resource.MustParse("0"),
}),
),
wantErrMsg: noError,
},
{
name: `validation of spec.jetStream.memStorage fails if enabled is true but size is 0`,
givenNATS: testutils.NewNATSCR(
testutils.WithNATSMemStorage(v1alpha1.MemStorage{
Enabled: true,
Size: resource.MustParse("0"),
}),
testutils.WithNATSResources(corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"memory": resource.MustParse("64Mi"),
},
spec: map[string]any{
jetStream: map[string]any{
memStorage: map[string]any{
enabled: false,
size: 0,
},
},
}),
),
wantErrMsg: "can only be enabled if size is not 0",
},
{
name: `validation of spec.jetStream.memStorage passes if enabled is true ` +
` and size is less than resources.limits.memory`,
givenNATS: testutils.NewNATSCR(
testutils.WithNATSResources(corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"memory": resource.MustParse("64Mi"),
},
},
},
}),
testutils.WithNATSMemStorage(v1alpha1.MemStorage{
Enabled: true,
Size: resource.MustParse("32Mi"),
}),
),
wantErrMsg: noError,
},
{
name: `validation of spec.jetStream.memStorage fails if enabled is true but size is 0`,
givenUnstructuredNATS: unstructured.Unstructured{
Object: map[string]any{
kind: kindNATS,
apiVersion: apiVersionNATS,
metadata: map[string]any{
name: testutils.GetRandK8sName(7),
namespace: testutils.GetRandK8sName(7),
name: `validation of spec.jetStream.memStorage fails if enabled is true ` +
` and size is greater than resources.limits.memory`,
givenNATS: testutils.NewNATSCR(
testutils.WithNATSResources(corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"memory": resource.MustParse("64Mi"),
},
spec: map[string]any{
jetStream: map[string]any{
memStorage: map[string]any{
enabled: true,
size: 0,
},
},
}),
testutils.WithNATSMemStorage(v1alpha1.MemStorage{
Enabled: true,
Size: resource.MustParse("1Gi"),
}),
),
wantErrMsg: "memStorage.size should be less than resources.limits.memory.",
},
{
name: `validation of spec.jetStream.memStorage passes if enabled is false ` +
` even if size is greater than resources.limits.memory`,
givenNATS: testutils.NewNATSCR(
testutils.WithNATSResources(corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"memory": resource.MustParse("64Mi"),
},
},
},
wantErrMsg: "can only be enabled if size is not 0",
}),
testutils.WithNATSMemStorage(v1alpha1.MemStorage{
Enabled: true,
Size: resource.MustParse("1Gi"),
}),
),
wantErrMsg: noError,
},
}

Expand All @@ -206,15 +191,19 @@ func Test_Validate_CreateNATS(t *testing.T) {
t.Parallel()

// given
testEnvironment.EnsureNamespaceCreation(t, tc.givenUnstructuredNATS.GetNamespace())
testEnvironment.EnsureNamespaceCreation(t, tc.givenNATS.GetNamespace())

// when
err := testEnvironment.CreateUnstructuredK8sResource(&tc.givenUnstructuredNATS)
err := testEnvironment.CreateK8sResource(tc.givenNATS)

//ss := resource.MustParse("0")

Check failure on line 199 in internal/controller/nats/integrationtests/validation/integration_test.go

View workflow job for this annotation

GitHub Actions / lint 

commentFormatting: put a space between `//` and comment text (gocritic)
//require.Equal(t, "0", ss.Size())

// then
if tc.wantErrMsg == noError {
require.NoError(t, err)
} else {
require.Error(t, err)
require.Contains(t, err.Error(), tc.wantErrMsg)
}
})
Expand Down

0 comments on commit 2c0fc82

Please sign in to comment.