diff --git a/Taskfile.yaml b/Taskfile.yaml index 85af936..9767952 100644 --- a/Taskfile.yaml +++ b/Taskfile.yaml @@ -9,7 +9,7 @@ includes: NESTED_MODULES: api API_DIRS: '{{.ROOT_DIR}}/api/...' MANIFEST_OUT: '{{.ROOT_DIR}}/api/crds/manifests' - CODE_DIRS: '{{.ROOT_DIR}}/cmd/... {{.ROOT_DIR}}/internal/... {{.ROOT_DIR}}/api/...' + CODE_DIRS: '{{.ROOT_DIR}}/cmd/... {{.ROOT_DIR}}/internal/... {{.ROOT_DIR}}/api/... {{.ROOT_DIR}}/pkg/...' COMPONENTS: platform-service-gateway REPO_URL: 'https://github.com/openmcp-project/platform-service-gateway' GENERATE_DOCS_INDEX: "false" diff --git a/internal/controllers/cluster/controller_test.go b/internal/controllers/cluster/controller_test.go new file mode 100644 index 0000000..f022f57 --- /dev/null +++ b/internal/controllers/cluster/controller_test.go @@ -0,0 +1,161 @@ +package cluster + +import ( + "testing" + + v1alpha1 "github.com/openmcp-project/openmcp-operator/api/clusters/v1alpha1" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + gatewayv1alpha1 "github.com/openmcp-project/platform-service-gateway/api/gateway/v1alpha1" +) + +var ( + terms = []gatewayv1alpha1.ClusterTerm{ + { + Selector: &gatewayv1alpha1.ClusterSelector{ + MatchPurpose: "platform", + }, + }, + { + Selector: &gatewayv1alpha1.ClusterSelector{ + MatchLabels: map[string]string{ + "gateway": "true", + }, + }, + }, + { + Selector: &gatewayv1alpha1.ClusterSelector{ + MatchLabels: map[string]string{ + "webhooks": "true", + }, + MatchPurpose: "workload", + }, + }, + { + ClusterRef: &gatewayv1alpha1.ClusterRef{ + Name: "foo", + Namespace: "bar", + }, + }, + } +) + +func Test_shouldReconcile(t *testing.T) { + testCases := []struct { + desc string + cluster *v1alpha1.Cluster + expected bool + }{ + { + desc: "should reconcile cluster with matching purpose", + cluster: &v1alpha1.Cluster{ + Spec: v1alpha1.ClusterSpec{ + Purposes: []string{"platform"}, + }, + }, + expected: true, + }, + { + desc: "should reconcile cluster with matching labels", + cluster: &v1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "foo": "bar", + "gateway": "true", + }, + }, + }, + expected: true, + }, + { + desc: "should reconcile cluster with matching labels and purpose", + cluster: &v1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "webhooks": "true", + }, + }, + Spec: v1alpha1.ClusterSpec{ + Purposes: []string{"workload"}, + }, + }, + expected: true, + }, + { + desc: "should not reconcile cluster with matching labels but wrong purpose", + cluster: &v1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "webhooks": "true", + }, + }, + Spec: v1alpha1.ClusterSpec{ + Purposes: []string{"mcp"}, + }, + }, + expected: false, + }, + { + desc: "should not reconcile cluster with matching purpose but wrong labels", + cluster: &v1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "foo": "bar", + }, + }, + Spec: v1alpha1.ClusterSpec{ + Purposes: []string{"workload"}, + }, + }, + expected: false, + }, + { + desc: "should reconcile cluster with matching ref", + cluster: &v1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + }, + expected: true, + }, + { + desc: "should not reconcile cluster with wrong ref", + cluster: &v1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "other", + }, + }, + expected: false, + }, + { + desc: "should reconcile cluster with wrong ref but has finalizer", + cluster: &v1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "other", + Finalizers: []string{ + gatewayv1alpha1.GatewayFinalizerOnCluster, + }, + }, + }, + expected: true, + }, + } + for _, tC := range testCases { + t.Run(tC.desc, func(t *testing.T) { + r := &ClusterReconciler{ + Config: &gatewayv1alpha1.GatewayServiceConfig{ + Spec: gatewayv1alpha1.GatewayServiceConfigSpec{ + Clusters: terms, + }, + }, + } + + actual := r.shouldReconcile(tC.cluster) + assert.Equal(t, tC.expected, actual) + }) + } +} diff --git a/pkg/envoy/config.go b/pkg/envoy/config.go index eef4479..5ebf2fe 100644 --- a/pkg/envoy/config.go +++ b/pkg/envoy/config.go @@ -6,7 +6,6 @@ import ( "fmt" "time" - "github.com/openmcp-project/platform-service-gateway/pkg/utils" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -16,6 +15,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/openmcp-project/platform-service-gateway/pkg/utils" ) var ( diff --git a/pkg/envoy/deployment.go b/pkg/envoy/deployment.go index 8c4e745..4b1323c 100644 --- a/pkg/envoy/deployment.go +++ b/pkg/envoy/deployment.go @@ -11,11 +11,12 @@ import ( fluxmeta "github.com/fluxcd/pkg/apis/meta" sourcev1 "github.com/fluxcd/source-controller/api/v1" clustersv1alpha1 "github.com/openmcp-project/openmcp-operator/api/clusters/v1alpha1" - "github.com/openmcp-project/platform-service-gateway/api/gateway/v1alpha1" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/openmcp-project/platform-service-gateway/api/gateway/v1alpha1" ) var ( diff --git a/pkg/utils/errors.go b/pkg/utils/errors.go index b5309a0..c2d5471 100644 --- a/pkg/utils/errors.go +++ b/pkg/utils/errors.go @@ -108,21 +108,3 @@ func IsCRDNotFoundError(err error) bool { return true } - -// IgnoreCRDNotFound returns nil on "CRD not found" errors. -// All other values that are not "CRD not found" errors or nil are returned unmodified. -func IgnoreCRDNotFound(err error) error { - if IsCRDNotFoundError(err) { - return nil - } - return err -} - -// IgnoreObjectOrCRDNotFound returns nil on NotFound and "CRD not found" errors. -// All other values that are not NotFound or "CRD not found" errors or nil are returned unmodified. -func IgnoreObjectOrCRDNotFound(err error) error { - if apierrors.IsNotFound(err) || IsCRDNotFoundError(err) { - return nil - } - return err -} diff --git a/pkg/utils/errors_test.go b/pkg/utils/errors_test.go new file mode 100644 index 0000000..ef986db --- /dev/null +++ b/pkg/utils/errors_test.go @@ -0,0 +1,124 @@ +package utils + +import ( + "errors" + "io/fs" + "testing" + "time" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" +) + +func TestRetryableError(t *testing.T) { + wrapped := errors.New("example error") + err := NewRetryableError(wrapped, time.Minute) + + assert.IsType(t, &RetryableError{}, err) + assert.True(t, errors.Is(err, &RetryableError{}), "error is not RetryableError") + assert.True(t, errors.Is(err, wrapped), "RetryableError does not contain wrapped error") + assert.False(t, errors.Is(err, fs.ErrClosed), "RetryableError matches unrelated error") + + re := &RetryableError{} + if assert.True(t, errors.As(err, &re)) { + assert.Equal(t, wrapped, re.Err) + assert.Equal(t, wrapped.Error(), re.Error()) + assert.Equal(t, time.Minute, re.RequeueAfter) + } +} + +func TestRemainingResourcesError(t *testing.T) { + objs := []client.Object{ + &corev1.Namespace{ + ObjectMeta: v1.ObjectMeta{ + Name: "example", + }, + }, + &corev1.Secret{ + ObjectMeta: v1.ObjectMeta{ + Name: "foo", + Namespace: "example", + }, + }, + } + err := NewRemainingResourcesError(time.Minute, objs...) + + assert.True(t, errors.Is(err, &RemainingResourcesError{}), "error is not RemainingResourcesError") + assert.True(t, errors.Is(err, &RetryableError{}), "error is not RetryableError") + assert.False(t, errors.Is(err, fs.ErrClosed), "RemainingResourcesError matches unrelated error") + + rr := &RemainingResourcesError{} + if assert.True(t, errors.As(err, &rr)) { + assert.EqualValues(t, objs, rr.Objects) + assert.Equal(t, "deletion of the following resources is still pending: [Namespace/example, Secret/example/foo]", rr.Error()) + } + + re := &RetryableError{} + if assert.True(t, errors.As(err, &re)) { + assert.Equal(t, time.Minute, re.RequeueAfter) + } +} + +func TestIsCRDNotFoundError(t *testing.T) { + testCases := []struct { + desc string + err error + expected bool + }{ + { + desc: "should return false when err is nil", + err: nil, + expected: false, + }, + { + desc: "should return false when err is NoResourceMatchError", + err: &meta.NoResourceMatchError{}, + expected: true, + }, + { + desc: "should return false when err is NoKindMatchError", + err: &meta.NoKindMatchError{}, + expected: true, + }, + { + desc: "resource discovery failed error", + err: &apiutil.ErrResourceDiscoveryFailed{}, + expected: true, + }, + { + desc: "not found error", + err: &apiutil.ErrResourceDiscoveryFailed{ + schema.GroupVersion{}: apierrors.NewNotFound(schema.GroupResource{}, ""), + }, + expected: true, + }, + { + desc: "multiple not found error", + err: &apiutil.ErrResourceDiscoveryFailed{ + schema.GroupVersion{}: apierrors.NewNotFound(schema.GroupResource{}, ""), + schema.GroupVersion{}: apierrors.NewNotFound(schema.GroupResource{}, ""), + }, + expected: true, + }, + { + desc: "Not found and forbidden error", + err: &apiutil.ErrResourceDiscoveryFailed{ + schema.GroupVersion{}: apierrors.NewNotFound(schema.GroupResource{}, ""), + schema.GroupVersion{}: apierrors.NewForbidden(schema.GroupResource{}, "", nil), + }, + expected: false, + }, + } + for _, tC := range testCases { + t.Run(tC.desc, func(t *testing.T) { + actual := IsCRDNotFoundError(tC.err) + assert.Equal(t, tC.expected, actual) + }) + } +} diff --git a/pkg/utils/meta_test.go b/pkg/utils/meta_test.go index 7961984..4391398 100644 --- a/pkg/utils/meta_test.go +++ b/pkg/utils/meta_test.go @@ -39,7 +39,7 @@ func TestObjectIdentifier(t *testing.T) { { desc: "EnvoyProxy (unstructured)", obj: unstructuredObj(), - expected: "Gateway/bar/foo", + expected: "EnvoyProxy/bar/foo", }, } for _, tC := range testCases {