Skip to content

Commit

Permalink
fix CEL peering validation and tests
Browse files Browse the repository at this point in the history
Signed-off-by: jose.vazquez <[email protected]>
  • Loading branch information
josvazg committed Feb 10, 2025
1 parent fbcc4fa commit 9e34cbf
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 8 deletions.
4 changes: 2 additions & 2 deletions api/v1/atlasnetworkpeering_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ type AtlasNetworkPeeringList struct {

// +kubebuilder:validation:XValidation:rule="(has(self.externalProjectRef) && !has(self.projectRef)) || (!has(self.externalProjectRef) && has(self.projectRef))",message="must define only one project reference through externalProjectRef or projectRef"
// +kubebuilder:validation:XValidation:rule="(has(self.externalProjectRef) && has(self.connectionSecret)) || !has(self.externalProjectRef)",message="must define a local connection secret when referencing an external project"
// +kubebuilder:validation:XValidation:rule="(has(self.containerRef.name) && !has(self.containerRef.id)) || (!has(self.containerRef.name) && has(self.containerRef.id))",message="must either a container Atlas id or Kubernetes name, but not both"
// +kubebuilder:validation:XValidation:rule="(self.containerRef.name != '' && self.containerRef.id == '') || (self.containerRef.name == '' && self.containerRef.id != '')",message="must either have a container Atlas id or Kubernetes name, but not both (or neither)"

// AtlasNetworkPeeringSpec defines the desired state of AtlasNetworkPeering
type AtlasNetworkPeeringSpec struct {
Expand All @@ -74,7 +74,7 @@ type ContainerDualReference struct {
// +optional
Name string `json:"name"`

// ID is teh Atlas identifier of the Network Container Atlas resource this Peering Connection relies on
// ID is the Atlas identifier of the Network Container Atlas resource this Peering Connection relies on
// Use either name or ID, not both.
// +optional
ID string `json:"id"`
Expand Down
77 changes: 77 additions & 0 deletions api/v1/atlasnetworkpeering_types_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package v1

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/runtime"

"github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1/common"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/test/helper/cel"
)

func TestPeeringCELChecks(t *testing.T) {
for _, tc := range []struct {
title string
obj *AtlasNetworkPeering
expectedErrors []string
}{
{
title: "Missing container ref in peering fails",
obj: &AtlasNetworkPeering{
Spec: AtlasNetworkPeeringSpec{},
},
expectedErrors: []string{"spec: Invalid value: \"object\": must either have a container Atlas id or Kubernetes name, but not both (or neither)"},
},

{
title: "Named container ref works",
obj: &AtlasNetworkPeering{
Spec: AtlasNetworkPeeringSpec{
ContainerRef: ContainerDualReference{
Name: "Some-name",
},
},
},
},

{
title: "Container id ref works",
obj: &AtlasNetworkPeering{
Spec: AtlasNetworkPeeringSpec{
ContainerRef: ContainerDualReference{
ID: "some-id",
},
},
},
},

{
title: "Both container id and name ref fails",
obj: &AtlasNetworkPeering{
Spec: AtlasNetworkPeeringSpec{
ContainerRef: ContainerDualReference{
Name: "Some-name",
ID: "some-id",
},
},
},
expectedErrors: []string{"spec: Invalid value: \"object\": must either have a container Atlas id or Kubernetes name, but not both (or neither)"},
},
} {
t.Run(tc.title, func(t *testing.T) {
// inject a project to avoid other CEL validations being hit
tc.obj.Spec.ProjectRef = &common.ResourceRefNamespaced{Name: "some-project"}
unstructuredObject, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&tc.obj)
require.NoError(t, err)

crdPath := "../../config/crd/bases/atlas.mongodb.com_atlasnetworkpeerings.yaml"
validator, err := cel.VersionValidatorFromFile(t, crdPath, "v1")
assert.NoError(t, err)
errs := validator(unstructuredObject, nil)

require.Equal(t, tc.expectedErrors, cel.ErrorListAsStrings(errs))
})
}
}
6 changes: 5 additions & 1 deletion api/v1/project_reference_cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@ var dualRefCRDs = []struct {
filename: "atlas.mongodb.com_atlasnetworkcontainers.yaml",
},
{
obj: &AtlasNetworkPeering{},
obj: &AtlasNetworkPeering{
Spec: AtlasNetworkPeeringSpec{ // Avoid triggering peering specific validations
ContainerRef: ContainerDualReference{Name: "fake-ref"},
},
},
filename: "atlas.mongodb.com_atlasnetworkpeerings.yaml",
},
}
Expand Down
10 changes: 5 additions & 5 deletions config/crd/bases/atlas.mongodb.com_atlasnetworkpeerings.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ spec:
properties:
id:
description: |-
ID is teh Atlas identifier of the Network Container Atlas resource this Peering Connection relies on
ID is the Atlas identifier of the Network Container Atlas resource this Peering Connection relies on
Use either name or ID, not both.
type: string
name:
Expand Down Expand Up @@ -182,10 +182,10 @@ spec:
project
rule: (has(self.externalProjectRef) && has(self.connectionSecret)) ||
!has(self.externalProjectRef)
- message: must either a container Atlas id or Kubernetes name, but not
both
rule: (has(self.containerRef.name) && !has(self.containerRef.id)) ||
(!has(self.containerRef.name) && has(self.containerRef.id))
- message: must either have a container Atlas id or Kubernetes name, but
not both (or neither)
rule: (self.containerRef.name != '' && self.containerRef.id == '') ||
(self.containerRef.name == '' && self.containerRef.id != '')
status:
description: |-
AtlasNetworkPeeringStatus is a status for the AtlasNetworkPeering Custom resource.
Expand Down

0 comments on commit 9e34cbf

Please sign in to comment.