Skip to content

Commit

Permalink
[DEVHAS-437] Move HAS-specific webhooks to application-service (#394)
Browse files Browse the repository at this point in the history
* Move Application-API webhooks to HAS

Signed-off-by: John Collier <jcollier@redhat.com>

* [DEVHAS-437] Move HAS webhooks in to application-service

Signed-off-by: John Collier <jcollier@redhat.com>

* Fix licenses

Signed-off-by: John Collier <jcollier@redhat.com>

* Cleanup

Signed-off-by: John Collier <jcollier@redhat.com>

* Fix webhook config

Signed-off-by: John Collier <jcollier@redhat.com>

---------

Signed-off-by: John Collier <jcollier@redhat.com>
  • Loading branch information
johnmcollier authored Oct 3, 2023
1 parent d41ddff commit 1c619fb
Show file tree
Hide file tree
Showing 14 changed files with 1,273 additions and 28 deletions.
8 changes: 8 additions & 0 deletions PROJECT
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,19 @@ resources:
group: appstudio
kind: Application
version: v1alpha1
webhooks:
defaulting: true
validation: true
webhookVersion: v1
- controller: true
domain: redhat.com
group: appstudio
kind: Component
version: v1alpha1
webhooks:
defaulting: true
validation: true
webhookVersion: v1
- controller: true
domain: redhat.com
group: appstudio
Expand Down
6 changes: 0 additions & 6 deletions config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

---
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
Expand All @@ -8,7 +7,6 @@ metadata:
webhooks:
- admissionReviewVersions:
- v1
- v1beta1
clientConfig:
service:
name: webhook-service
Expand All @@ -29,7 +27,6 @@ webhooks:
sideEffects: None
- admissionReviewVersions:
- v1
- v1beta1
clientConfig:
service:
name: webhook-service
Expand All @@ -48,7 +45,6 @@ webhooks:
resources:
- components
sideEffects: None

---
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
Expand All @@ -58,7 +54,6 @@ metadata:
webhooks:
- admissionReviewVersions:
- v1
- v1beta1
clientConfig:
service:
name: webhook-service
Expand All @@ -79,7 +74,6 @@ webhooks:
sideEffects: None
- admissionReviewVersions:
- v1
- v1beta1
clientConfig:
service:
name: webhook-service
Expand Down
2 changes: 1 addition & 1 deletion contracts/application_pact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestContracts(t *testing.T) {
BrokerURL: "https://pact-broker-hac-pact-broker.apps.hac-devsandbox.5unc.p1.openshiftapps.com",
PublishVerificationResults: false,
BrokerUsername: "pactCommonUser",
BrokerPassword: "pactCommonPassword123",
BrokerPassword: "pactCommonPassword123", // notsecret
}

if os.Getenv("SKIP_PACT_TESTS") == "true" {
Expand Down
90 changes: 90 additions & 0 deletions controllers/webhooks/application_webhook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
Copyright 2022 Red Hat, Inc.
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 webhooks

import (
"context"
"fmt"
"reflect"

"github.com/go-logr/logr"
appstudiov1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// Webhook describes the data structure for the release webhook
type ApplicationWebhook struct {
client client.Client
log logr.Logger
}

//+kubebuilder:webhook:path=/mutate-appstudio-redhat-com-v1alpha1-application,mutating=true,failurePolicy=fail,sideEffects=None,groups=appstudio.redhat.com,resources=applications,verbs=create;update,versions=v1alpha1,name=mapplication.kb.io,admissionReviewVersions=v1

func (w *ApplicationWebhook) Register(mgr ctrl.Manager, log *logr.Logger) error {
w.client = mgr.GetClient()

return ctrl.NewWebhookManagedBy(mgr).
For(&appstudiov1alpha1.Application{}).
WithValidator(w).
Complete()
}

// +kubebuilder:webhook:path=/validate-appstudio-redhat-com-v1alpha1-application,mutating=false,failurePolicy=fail,sideEffects=None,groups=appstudio.redhat.com,resources=applications,verbs=create;update,versions=v1alpha1,name=vapplication.kb.io,admissionReviewVersions=v1

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (r *ApplicationWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) error {
app := obj.(*appstudiov1alpha1.Application)

applicationlog := r.log.WithValues("controllerKind", "Application").WithValues("name", app.Name).WithValues("namespace", app.Namespace)
applicationlog.Info("validating the create request")
// We use the DNS-1035 format for application names, so ensure it conforms to that specification
if len(validation.IsDNS1035Label(app.Name)) != 0 {
return fmt.Errorf("invalid application name: %q: an application resource name must start with a lower case alphabetical character, be under 63 characters, and can only consist of lower case alphanumeric characters or ‘-’,", app.Name)
}
if app.Spec.DisplayName == "" {
return fmt.Errorf("display name must be provided when creating an Application")
}
return nil
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (r *ApplicationWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error {
oldApp := oldObj.(*appstudiov1alpha1.Application)
newApp := newObj.(*appstudiov1alpha1.Application)
applicationlog := r.log.WithValues("controllerKind", "Application").WithValues("name", newApp.Name).WithValues("namespace", newApp.Namespace)
applicationlog.Info("validating the update request")

if !reflect.DeepEqual(newApp.Spec.AppModelRepository, oldApp.Spec.AppModelRepository) {
return fmt.Errorf("app model repository cannot be updated to %+v", newApp.Spec.AppModelRepository)
}

if !reflect.DeepEqual(newApp.Spec.GitOpsRepository, oldApp.Spec.GitOpsRepository) {
return fmt.Errorf("gitops repository cannot be updated to %+v", newApp.Spec.GitOpsRepository)
}

return nil
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (r *ApplicationWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) error {

// TODO(user): fill in your validation logic upon object deletion.
return nil
}
178 changes: 178 additions & 0 deletions controllers/webhooks/application_webhook_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
//
// Copyright 2022 Red Hat, Inc.
//
// 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 webhooks

import (
"context"
"reflect"
"time"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

appstudiov1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
//+kubebuilder:scaffold:imports
)

const (
timeout = time.Second * 10
duration = time.Second * 10
interval = time.Millisecond * 250
)

var _ = Describe("Application validation webhook", func() {

// Define utility constants for object names and testing timeouts/durations and intervals.
const (
HASAppName = "test-application"
HASAppNamespace = "default"
DisplayName = "petclinic"
Description = "Simple petclinic app"
)

Context("Create Application CR with missing displayName", func() {
It("Should fail with error saying displayName is required field", func() {
ctx := context.Background()

hasApp := &appstudiov1alpha1.Application{
TypeMeta: metav1.TypeMeta{
APIVersion: "appstudio.redhat.com/v1alpha1",
Kind: "Application",
},
ObjectMeta: metav1.ObjectMeta{
Name: HASAppName,
Namespace: HASAppNamespace,
},
Spec: appstudiov1alpha1.ApplicationSpec{
AppModelRepository: appstudiov1alpha1.ApplicationGitRepository{
URL: "https://github.com/testorg/petclinic-app",
},
GitOpsRepository: appstudiov1alpha1.ApplicationGitRepository{
URL: "https://github.com/testorg/gitops-app",
},
},
}

Expect(k8sClient.Create(ctx, hasApp)).Should(Not(Succeed()))
})
})

Context("Create Application CR with invalid metadata.name", func() {
It("Should fail with error saying name does not conform to spec", func() {
ctx := context.Background()

hasApp := &appstudiov1alpha1.Application{
TypeMeta: metav1.TypeMeta{
APIVersion: "appstudio.redhat.com/v1alpha1",
Kind: "Application",
},
ObjectMeta: metav1.ObjectMeta{
Name: "1-invalid-application-name",
Namespace: HASAppNamespace,
},
Spec: appstudiov1alpha1.ApplicationSpec{
AppModelRepository: appstudiov1alpha1.ApplicationGitRepository{
URL: "https://github.com/testorg/petclinic-app",
},
GitOpsRepository: appstudiov1alpha1.ApplicationGitRepository{
URL: "https://github.com/testorg/gitops-app",
},
},
}

err := k8sClient.Create(ctx, hasApp)
Expect(err).Should(Not(Succeed()))
Expect(err.Error()).Should(ContainSubstring("an application resource name must start with a lower case alphabetical character, be under 63 characters, and can only consist of lower case alphanumeric characters or ‘-’"))
})
})

Context("Update Application CR fields", func() {
It("Should update non immutable fields successfully and err out on immutable fields", func() {
ctx := context.Background()

hasApp := &appstudiov1alpha1.Application{
TypeMeta: metav1.TypeMeta{
APIVersion: "appstudio.redhat.com/v1alpha1",
Kind: "Application",
},
ObjectMeta: metav1.ObjectMeta{
Name: HASAppName,
Namespace: HASAppNamespace,
},
Spec: appstudiov1alpha1.ApplicationSpec{
DisplayName: DisplayName,
Description: Description,
AppModelRepository: appstudiov1alpha1.ApplicationGitRepository{
URL: "https://github.com/testorg/petclinic-app",
},
GitOpsRepository: appstudiov1alpha1.ApplicationGitRepository{
URL: "https://github.com/testorg/gitops-app",
},
},
}

Expect(k8sClient.Create(ctx, hasApp)).Should(Succeed())

// Look up the has app resource that was created.
// These tests do not go through reconcile, so no need to check reconcile logic here
hasAppLookupKey := types.NamespacedName{Name: HASAppName, Namespace: HASAppNamespace}
fetchedHasApp := &appstudiov1alpha1.Application{}
Eventually(func() bool {
k8sClient.Get(ctx, hasAppLookupKey, fetchedHasApp)
return !reflect.DeepEqual(fetchedHasApp, &appstudiov1alpha1.Application{})
}, timeout, interval).Should(BeTrue())

// Update the hasApp resource
fetchedHasApp.Spec.DisplayName = "newname"
fetchedHasApp.Spec.Description = "New Description"
fetchedHasApp.Spec.AppModelRepository.URL = "newurl"
err := k8sClient.Update(ctx, fetchedHasApp)
Expect(err).Should(HaveOccurred())
Expect(err.Error()).Should(ContainSubstring("app model repository cannot be updated"))

// revert appmodel but update gitops
fetchedHasApp.Spec.AppModelRepository.URL = "https://github.com/testorg/petclinic-app"
fetchedHasApp.Spec.GitOpsRepository.URL = "https://github.com/testorg/petclinic-app"

err = k8sClient.Update(ctx, fetchedHasApp)
Expect(err).Should(HaveOccurred())
Expect(err.Error()).Should(ContainSubstring("gitops repository cannot be updated"))

// Delete the specified resource
deleteHASAppCR(hasAppLookupKey)
})
})

})

// deleteHASAppCR deletes the specified hasApp resource and verifies it was properly deleted
func deleteHASAppCR(hasAppLookupKey types.NamespacedName) {
// Delete
Eventually(func() error {
f := &appstudiov1alpha1.Application{}
k8sClient.Get(context.Background(), hasAppLookupKey, f)
return k8sClient.Delete(context.Background(), f)
}, timeout, interval).Should(Succeed())

// Wait for delete to finish
Eventually(func() error {
f := &appstudiov1alpha1.Application{}
return k8sClient.Get(context.Background(), hasAppLookupKey, f)
}, timeout, interval).ShouldNot(Succeed())
}
Loading

0 comments on commit 1c619fb

Please sign in to comment.