diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 938a666..b5376c9 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -18,25 +18,25 @@ jobs: with: version: latest args: --timeout 5m - # test: - # name: test - # runs-on: ubuntu-latest - # steps: - # - uses: actions/checkout@v2 - # - uses: actions/setup-go@v2 - # with: - # go-version: '^1.16' - # - uses: RyanSiu1995/kubebuilder-action@v1.2.1 - # - run: make test - # - uses: codecov/codecov-action@v1 - # with: - # files: coverage.out + test: + name: test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-go@v2 + with: + go-version: '^1.16' + - uses: RyanSiu1995/kubebuilder-action@v1.2.1 + - run: make test + - uses: codecov/codecov-action@v1 + with: + files: coverage.out docker: name: docker runs-on: ubuntu-latest needs: - lint - # - test + - test steps: - uses: actions/checkout@v2 - uses: docker/setup-qemu-action@v1 diff --git a/api/v1alpha1/team_webhook.go b/api/v1alpha1/team_webhook.go index 0d692c3..17aed38 100644 --- a/api/v1alpha1/team_webhook.go +++ b/api/v1alpha1/team_webhook.go @@ -51,25 +51,26 @@ var _ webhook.Validator = &Team{} var teamns corev1.Namespace func (t *Team) ValidateCreate() error { - teamlog.Info("validate create", "name", t.Name) + teamlog.Info("validating team create", "name", t.GetName()) clientSet, err := getClient() if err != nil { + teamlog.Error(err, "error happened while validating create", "namespace", t.GetNamespace(), "name", t.GetName()) return errors.New("could not create client, failed to update team object") } for _, ns := range t.Spec.Namespaces { - //check if namespace does not exist or has been deleted - teamns, err = nsExists(ns, clientSet) + // Check if namespace does not exist or has been deleted + teamns, err = nsExists(clientSet, t.Name, ns) if err != nil { return err } - //check if namespace already has been added to another team + // Check if namespace already has been added to another team err = nsHasTeam(t, &teamns) if err != nil { return err } - //Check If user has access to this namespace + // Check If user has access to this namespace err = teamAdminAccess(t, ns, clientSet) if err != nil { return err @@ -79,15 +80,16 @@ func (t *Team) ValidateCreate() error { } func (t *Team) ValidateUpdate(old runtime.Object) error { - teamlog.Info("validate update", "name", t.Name) + teamlog.Info("validating team update", "name", t.GetName()) clientSet, err := getClient() if err != nil { + teamlog.Error(err, "error happened while validating update", "namespace", t.GetNamespace(), "name", t.GetName()) return errors.New("fail to get client, failed to update team object") } for _, ns := range t.Spec.Namespaces { //check if namespace does not exist or has been deleted - teamns, err = nsExists(ns, clientSet) + teamns, err = nsExists(clientSet, t.Name, ns) if err != nil { return err } @@ -126,7 +128,7 @@ func (t *Team) ValidateUpdate(old runtime.Object) error { } } if !exists { - errMessage := fmt.Sprintf("namespace %s has team label but does not exist in %s team", ni.Name, t.Name) + errMessage := fmt.Sprintf("namespace \"%s\" has team label but does not exist in \"%s\" team", ni.Name, t.Name) return errors.New(errMessage) } } @@ -153,11 +155,11 @@ func getClient() (c kubernetes.Clientset, err error) { return *clientSet, nil } -func nsExists(ns string, c kubernetes.Clientset) (tns corev1.Namespace, err error) { +func nsExists(c kubernetes.Clientset, team, ns string) (tns corev1.Namespace, err error) { teamNS, errNSGet := c.CoreV1().Namespaces().Get(context.TODO(), ns, metav1.GetOptions{}) if errNSGet != nil { - errorResp := fmt.Sprintf("Error while getting namespace %s: %s", ns, errNSGet.Error()) + errorResp := fmt.Sprintf("Error while getting namespace \"%s\" for team \"%s\". error: %s", ns, team, errNSGet.Error()) return *teamNS, errors.New(errorResp) } return *teamNS, nil @@ -166,7 +168,7 @@ func nsExists(ns string, c kubernetes.Clientset) (tns corev1.Namespace, err erro func nsHasTeam(r *Team, tns *corev1.Namespace) (err error) { if val, ok := tns.Labels["snappcloud.io/team"]; ok { if tns.Labels["snappcloud.io/team"] != r.Name { - errorResp := fmt.Sprintf("namespace %s already has the team label %s, please ask in cloud-support if you need to detach the namespace from previous team", tns.Name, val) + errorResp := fmt.Sprintf("namespace \"%s\" inside the Namespaces of team \"%s\" already has the team label \"%s\", please ask in cloud-support if you need to detach the namespace from previous team", tns.Name, r.Name, val) return errors.New(errorResp) } } @@ -194,10 +196,10 @@ func teamAdminAccess(r *Team, ns string, c kubernetes.Clientset) (err error) { Create(context.TODO(), &check, metav1.CreateOptions{}) if errAuth != nil { - teamlog.Error(errAuth, "team owner does not have permission to create role binding") + teamlog.Error(errAuth, "error happened while checking team owner permission") } if !resp.Status.Allowed { - errMessage := fmt.Sprintf("team owner is not allowed to add namespace %s, please add %s as admin of the project with the followig command: oc policy add-role-to-user admin %s -n %s", ns, r.Spec.TeamAdmin, r.Spec.TeamAdmin, ns) + errMessage := fmt.Sprintf("team owner \"%s\" is not allowed to add namespace \"%s\" to team \"%s\", please add \"%s\" as admin of the project with the followig command: oc policy add-role-to-user admin %s -n %s", r.Spec.TeamAdmin, ns, r.Name, r.Spec.TeamAdmin, r.Spec.TeamAdmin, ns) return errors.New(errMessage) } return nil diff --git a/api/v1alpha1/team_webhook_test.go b/api/v1alpha1/team_webhook_test.go index 8ca88e5..651108c 100644 --- a/api/v1alpha1/team_webhook_test.go +++ b/api/v1alpha1/team_webhook_test.go @@ -3,14 +3,19 @@ package v1alpha1 import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" ) var _ = Describe("", func() { var ( - fooTeamName = "foo-team" - fooTeamAdminName = "foo-admin" + fooTeamName = "foo-team" + fooTeamAdminName = "foo-admin" + fooTeamNamespaces = []string{"default"} ) var ( err error @@ -54,36 +59,45 @@ var _ = Describe("", func() { Expect(err).NotTo(BeNil()) }) - //It("should fail if TeamAdmin does not exist", func() { - // fooTeamTmp := &Team{ - // TypeMeta: fooTeam.TypeMeta, - // ObjectMeta: fooTeam.ObjectMeta, - // Spec: TeamSpec{ - // TeamAdmin: "not-existing-team-admin", - // Namespaces: fooTeamNamespaces, - // }, - // } - // err = k8sClient.Create(ctx, fooTeamTmp) - // Expect(err).NotTo(BeNil()) - //}) - // - //It("should fail if at least one namespace has team label from another team", func() { - // ns := &corev1.Namespace{} - // err = k8sClient.Get(ctx, types.NamespacedName{Name: fooTeamNamespaces[0]}, ns) - // Expect(err).To(BeNil()) - // patch := []byte(`{"metadata":{"labels":{"snappcloud.io/team": "non-existing-team"}}}`) - // err = k8sClient.Patch(ctx, ns, client.RawPatch(types.StrategicMergePatchType, patch)) - // Expect(err).To(BeNil()) - // fooTeamTmp := &Team{ - // TypeMeta: fooTeam.TypeMeta, - // ObjectMeta: fooTeam.ObjectMeta, - // Spec: TeamSpec{ - // TeamAdmin: fooTeamAdminName, - // Namespaces: fooTeamNamespaces, - // }, - // } - // err = k8sClient.Create(ctx, fooTeamTmp) - // Expect(err).NotTo(BeNil()) - //}) + It("should fail if TeamAdmin does not exist", func() { + ns := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-namespace", + }, + } + err = k8sClient.Create(ctx, ns) + Expect(err).To(BeNil()) + fooTeamTmp := &Team{ + TypeMeta: fooTeam.TypeMeta, + ObjectMeta: fooTeam.ObjectMeta, + Spec: TeamSpec{ + TeamAdmin: "not-existing-team-admin", + Namespaces: []string{ + "foo-namespace", + }, + }, + } + err = k8sClient.Create(ctx, fooTeamTmp) + Expect(err).NotTo(BeNil()) + }) + + It("should fail if at least one namespace has team label from another team", func() { + ns := &corev1.Namespace{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: fooTeamNamespaces[0]}, ns) + Expect(err).To(BeNil()) + patch := []byte(`{"metadata":{"labels":{"snappcloud.io/team": "non-existing-team"}}}`) + err = k8sClient.Patch(ctx, ns, client.RawPatch(types.StrategicMergePatchType, patch)) + Expect(err).To(BeNil()) + fooTeamTmp := &Team{ + TypeMeta: fooTeam.TypeMeta, + ObjectMeta: fooTeam.ObjectMeta, + Spec: TeamSpec{ + TeamAdmin: fooTeamAdminName, + Namespaces: fooTeamNamespaces, + }, + } + err = k8sClient.Create(ctx, fooTeamTmp) + Expect(err).NotTo(BeNil()) + }) }) }) diff --git a/api/v1alpha1/webhook_suite_test.go b/api/v1alpha1/webhook_suite_test.go index 2ae5778..e1b63c5 100644 --- a/api/v1alpha1/webhook_suite_test.go +++ b/api/v1alpha1/webhook_suite_test.go @@ -28,9 +28,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - admissionv1beta1 "k8s.io/api/admission/v1beta1" - //+kubebuilder:scaffold:imports - "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -69,28 +67,25 @@ var _ = BeforeSuite(func() { } var err error + + err = SchemeBuilder.AddToScheme(scheme.Scheme) + Expect(err).To(BeNil()) + // cfg is defined in this file globally. cfg, err = testEnv.Start() Expect(err).NotTo(HaveOccurred()) Expect(cfg).NotTo(BeNil()) - scheme := runtime.NewScheme() - err = AddToScheme(scheme) - Expect(err).NotTo(HaveOccurred()) - - err = admissionv1beta1.AddToScheme(scheme) - Expect(err).NotTo(HaveOccurred()) - //+kubebuilder:scaffold:scheme - k8sClient, err = client.New(cfg, client.Options{Scheme: scheme}) + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) Expect(err).NotTo(HaveOccurred()) Expect(k8sClient).NotTo(BeNil()) // start webhook server using Manager webhookInstallOptions := &testEnv.WebhookInstallOptions mgr, err := ctrl.NewManager(cfg, ctrl.Options{ - Scheme: scheme, + Scheme: scheme.Scheme, Host: webhookInstallOptions.LocalServingHost, Port: webhookInstallOptions.LocalServingPort, CertDir: webhookInstallOptions.LocalServingCertDir,