diff --git a/cmd/ketch/job_deploy.go b/cmd/ketch/job_deploy.go index 7fe461a9..9dc95f2f 100644 --- a/cmd/ketch/job_deploy.go +++ b/cmd/ketch/job_deploy.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "os" + "strings" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -56,13 +57,19 @@ func jobDeploy(ctx context.Context, cfg config, filename string, out io.Writer) } job := &ketchv1.Job{ObjectMeta: metav1.ObjectMeta{Name: spec.Name}} - _, err = controllerutil.CreateOrUpdate(ctx, cfg.Client(), job, func() error { + res, err := controllerutil.CreateOrUpdate(ctx, cfg.Client(), job, func() error { job.Spec = spec return nil }) if err != nil { + if strings.Contains(err.Error(), ketchv1.ErrJobExists.Error()) { + return ketchv1.ErrJobExists + } return err } + if res == controllerutil.OperationResultNone { + return fmt.Errorf("job \"%s\" already exists and is unchanged", job.Spec.Name) + } fmt.Fprintln(out, "Successfully added!") return nil diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 61f76fdb..7a6beeb9 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -135,6 +135,10 @@ func main() { setupLog.Error(err, "unable to create webhook", "webhook", "Framework") os.Exit(1) } + if err = (&ketchv1.Job{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "Job") + os.Exit(1) + } } // +kubebuilder:scaffold:builder diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index b59b50dd..fbdd619c 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -55,3 +55,24 @@ webhooks: resources: - frameworks sideEffects: None +- admissionReviewVersions: + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-theketch-io-v1beta1-job + failurePolicy: Fail + name: vjob.kb.io + rules: + - apiGroups: + - theketch.io + apiVersions: + - v1beta1 + operations: + - CREATE + - UPDATE + - DELETE + resources: + - jobs + sideEffects: None diff --git a/internal/api/v1beta1/errors.go b/internal/api/v1beta1/errors.go index 8103833f..3ae31eb1 100644 --- a/internal/api/v1beta1/errors.go +++ b/internal/api/v1beta1/errors.go @@ -23,4 +23,7 @@ const ( // ErrDecreaseQuota is returned when a new quota is too small. ErrDecreaseQuota Error = "failed to decrease quota because the framework has more running apps than the new quota permits" + + // ErrJobExists + ErrJobExists Error = "failed to create job because the job already exists" ) diff --git a/internal/api/v1beta1/job_webhook.go b/internal/api/v1beta1/job_webhook.go new file mode 100644 index 00000000..9543f1eb --- /dev/null +++ b/internal/api/v1beta1/job_webhook.go @@ -0,0 +1,84 @@ +/* + + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1 + +import ( + "context" + "fmt" + + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook" +) + +// joblog is for logging in this package. +var joblog = logf.Log.WithName("job-resource") + +var jobmgr manager = nil + +func (r *Job) SetupWebhookWithManager(mgr ctrl.Manager) error { + jobmgr = mgr + return ctrl.NewWebhookManagedBy(mgr). + For(r). + Complete() +} + +// +kubebuilder:webhook:verbs=create;update;delete,path=/validate-theketch-io-v1beta1-job,mutating=false,failurePolicy=fail,groups=theketch.io,resources=jobs,versions=v1beta1,name=vjob.kb.io,sideEffects=none,admissionReviewVersions=v1beta1 + +var _ webhook.Validator = &Job{} + +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +func (r *Job) ValidateCreate() error { + joblog.Info("validate create", "name", r.Name) + client := jobmgr.GetClient() + jobs := JobList{} + if err := client.List(context.Background(), &jobs); err != nil { + return err + } + for _, job := range jobs.Items { + if job.Spec.Name == r.Spec.Name { + return ErrJobExists + } + } + return nil +} + +// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type +func (r *Job) ValidateUpdate(old runtime.Object) error { + joblog.Info("validate update", "name", r.Name) + oldJob, ok := old.(*Job) + if !ok { + return fmt.Errorf("can't validate job update") + } + client := jobmgr.GetClient() + jobs := JobList{} + if err := client.List(context.Background(), &jobs); err != nil { + return err + } + for _, job := range jobs.Items { + if job.Spec.Name == oldJob.Spec.Name { + return ErrJobExists + } + } + return nil +} + +// ValidateDelete implements webhook.Validator so a webhook will be registered for the type +func (r *Job) ValidateDelete() error { + return nil +} diff --git a/internal/api/v1beta1/job_webhook_test.go b/internal/api/v1beta1/job_webhook_test.go new file mode 100644 index 00000000..e5ba8c54 --- /dev/null +++ b/internal/api/v1beta1/job_webhook_test.go @@ -0,0 +1,174 @@ +package v1beta1 + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/shipa-corp/ketch/internal/api/v1beta1/mocks" +) + +func TestJob_ValidateDelete(t *testing.T) { + tests := []struct { + name string + job Job + }{ + { + name: "success", + job: Job{ + Status: JobStatus{}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.job.ValidateDelete() + require.Nil(t, err) + }) + } +} + +func TestJob_ValidateCreate(t *testing.T) { + + const listError Error = "error" + + tests := []struct { + name string + job Job + client *mocks.MockClient + wantErr error + }{ + { + name: "error getting a list of jobs", + client: &mocks.MockClient{ + OnList: func(ctx context.Context, list runtime.Object, opts ...client.ListOption) error { + return listError + }, + }, + wantErr: listError, + }, + { + name: "job already exists", + client: &mocks.MockClient{ + OnList: func(ctx context.Context, list runtime.Object, opts ...client.ListOption) error { + jobs := list.(*JobList) + jobs.Items = []Job{ + {Spec: JobSpec{Name: "test-job"}}, + } + return nil + }, + }, + job: Job{ + Spec: JobSpec{ + Name: "test-job", + }, + }, + wantErr: ErrJobExists, + }, + { + name: "success", + client: &mocks.MockClient{ + OnList: func(ctx context.Context, list runtime.Object, opts ...client.ListOption) error { + jobs := list.(*JobList) + jobs.Items = []Job{ + {Spec: JobSpec{Name: "test-job"}}, + } + return nil + }, + }, + job: Job{ + Spec: JobSpec{ + Name: "another-test-job", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + jobmgr = &mockManager{client: tt.client} + if err := tt.job.ValidateCreate(); err != tt.wantErr { + t.Errorf("ValidateCreate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestJob_ValidateUpdate(t *testing.T) { + + const listError Error = "error" + + tests := []struct { + name string + job Job + old runtime.Object + client *mocks.MockClient + wantErr error + }{ + { + name: "error getting a list of jobs", + job: Job{ + Spec: JobSpec{Name: "test-job"}, + }, + client: &mocks.MockClient{ + OnList: func(ctx context.Context, list runtime.Object, opts ...client.ListOption) error { + return listError + }, + }, + old: &Job{ + Spec: JobSpec{Name: "test-job"}, + }, + wantErr: listError, + }, + { + name: "job already exists", + job: Job{ + ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, + Spec: JobSpec{Name: "test-job"}, + }, + client: &mocks.MockClient{ + OnList: func(ctx context.Context, list runtime.Object, opts ...client.ListOption) error { + jobs := list.(*JobList) + jobs.Items = []Job{ + {ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, Spec: JobSpec{Name: "test-job"}}, + } + return nil + }, + }, + old: &Job{ + Spec: JobSpec{Name: "test-job"}, + }, + wantErr: ErrJobExists, + }, + { + name: "everything is ok", + job: Job{ + ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, + Spec: JobSpec{Name: "another-job"}, + }, + client: &mocks.MockClient{ + OnList: func(ctx context.Context, list runtime.Object, opts ...client.ListOption) error { + jobs := list.(*JobList) + jobs.Items = []Job{ + {ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, Spec: JobSpec{Name: "another-job"}}, + } + return nil + }, + }, + old: &Job{ + Spec: JobSpec{Name: "test-job"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + jobmgr = &mockManager{client: tt.client} + if err := tt.job.ValidateUpdate(tt.old); err != tt.wantErr { + t.Errorf("ValidateUpdate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +}