Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce webhook to prevent more than 1 KIP resource in a single namespace #115

Merged
merged 19 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/next-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ jobs:
-
name: "Set up Docker Buildx"
uses: docker/setup-buildx-action@v2
-
name: Set up kubebuilder
uses: RyanSiu1995/[email protected]
with:
version: 2.3.2
-
name: "Docker quay.io Login"
uses: docker/login-action@v2
Expand Down
6 changes: 5 additions & 1 deletion .github/workflows/pr-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ jobs:
uses: actions/setup-go@v3
with:
go-version: 1.18
- name: Set up kubebuilder
uses: RyanSiu1995/[email protected]
with:
version: 2.3.2
dkwon17 marked this conversation as resolved.
Show resolved Hide resolved
- name: Run unit tests
run: make test
image-build:
Expand All @@ -30,7 +34,7 @@ jobs:
- name: Checkout source code
uses: actions/checkout@v3
- name: Build image
run: docker build -f build/Dockerfile .
run: docker build -f build/Dockerfile --build-arg SKIP_TESTS=true .
scorecard:
runs-on: ubuntu-22.04
steps:
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ jobs:
uses: actions/setup-go@v3
with:
go-version: 1.18
- name: Set up kubebuilder
uses: RyanSiu1995/[email protected]
with:
version: 2.3.2
- name: Release operator
run: |
RELEASE_VERSION=${{ github.event.inputs.version }}
Expand Down
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ bundle: generate manifests download-kustomize download-operator-sdk ## Generate

CSV_PATH=$$($(MAKE) csv-path)
yq -riY '.metadata.annotations.containerImage = "'$(IMG)'"' $${CSV_PATH}
yq -riY '.spec.install.spec.deployments[0].spec.template.spec.containers[1].image = "'$(IMG)'"' $${CSV_PATH}
# Update container image for container 'kuebrnetes-image-puller-operator' in the list of deployments
yq -riY '.spec.install.spec.deployments[0].spec.template.spec.containers[] |= (select(.name == "kubernetes-image-puller-operator") .image |= "'$(IMG)'")' $${CSV_PATH}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When running make bundle on the dogfooding cluster, the order of the containers are different. This change makes it so that we don't assume that the kubernetes-image-puller-operator container is defined at index 1


# Copy bundle.Dockerfile to the bundle dir
# Update paths (since it is created in the root of the project) and labels
Expand Down
6 changes: 5 additions & 1 deletion PROJECT
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
domain: eclipse.che
domain: eclipse.org
layout:
- go.kubebuilder.io/v3
plugins:
Expand All @@ -16,4 +16,8 @@ resources:
kind: KubernetesImagePuller
path: github.com/che-incubator/kubernetes-image-puller-operator/api/v1alpha1
version: v1alpha1
webhooks:
defaulting: true
validation: true
webhookVersion: v1
version: "3"
46 changes: 46 additions & 0 deletions api/v1alpha1/kubernetesimagepuller_webhook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
//
// Copyright (c) 2012-2023 Red Hat, Inc.
// This program and the accompanying materials are made
// available under the terms of the Eclipse Public License 2.0
// which is available at https://www.eclipse.org/legal/epl-2.0/
//
// SPDX-License-Identifier: EPL-2.0
//
// Contributors:
// Red Hat, Inc. - initial API and implementation
//

package v1alpha1
dkwon17 marked this conversation as resolved.
Show resolved Hide resolved

import (
"context"

ctrl "sigs.k8s.io/controller-runtime"

"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/webhook"
)

func (r *KubernetesImagePuller) SetupWebhookWithManager(mgr ctrl.Manager) error {
mgr.GetWebhookServer().Register("/validate-che-eclipse-org-v1alpha1-kubernetesimagepuller", &webhook.Admission{Handler: &validationHandler{k8sClient: mgr.GetClient()}})
return nil
}

// +kubebuilder:webhook:path=/validate-che-eclipse-org-v1alpha1-kubernetesimagepuller,mutating=false,failurePolicy=fail,sideEffects=None,groups=che.eclipse.org,resources=kubernetesimagepullers,verbs=create,versions=v1alpha1,name=vkubernetesimagepuller.kb.io,admissionReviewVersions={v1,v1beta1}

type validationHandler struct {
k8sClient client.Client
}

func (v *validationHandler) Handle(ctx context.Context, req webhook.AdmissionRequest) webhook.AdmissionResponse {
imagePullers := &KubernetesImagePullerList{}
err := v.k8sClient.List(ctx, imagePullers, &client.ListOptions{Namespace: req.Namespace})
if err != nil {
return webhook.Denied(err.Error())
}

if len(imagePullers.Items) > 0 {
return webhook.Denied("only one KubernetesImagePuller is allowed per namespace")
}
return webhook.Allowed("there are no KubernetesImagePuller resources in this namespace")
}
182 changes: 182 additions & 0 deletions api/v1alpha1/webhook_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
//
// Copyright (c) 2012-2023 Red Hat, Inc.
// This program and the accompanying materials are made
// available under the terms of the Eclipse Public License 2.0
// which is available at https://www.eclipse.org/legal/epl-2.0/
//
// SPDX-License-Identifier: EPL-2.0
//
// Contributors:
// Red Hat, Inc. - initial API and implementation
//

package v1alpha1

import (
"context"
"crypto/tls"
"fmt"
"net"
"path/filepath"
"testing"
"time"

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

admissionv1beta1 "k8s.io/api/admission/v1beta1"
v1 "k8s.io/api/core/v1"

//+kubebuilder:scaffold:imports
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/rest"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"
"sigs.k8s.io/controller-runtime/pkg/envtest/printer"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
)

// These tests use Ginkgo (BDD-style Go testing framework). Refer to
// http://onsi.github.io/ginkgo/ to learn more about Ginkgo.

var cfg *rest.Config
var k8sClient client.Client
var testEnv *envtest.Environment
var ctx context.Context
var cancel context.CancelFunc

func TestAPIs(t *testing.T) {
RegisterFailHandler(Fail)

RunSpecsWithDefaultAndCustomReporters(t,
"Webhook Suite",
[]Reporter{printer.NewlineReporter{}})
}

var _ = BeforeSuite(func() {
logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))

ctx, cancel = context.WithCancel(context.TODO())

By("bootstrapping test environment")
testEnv = &envtest.Environment{
CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")},
ErrorIfCRDPathMissing: false,
WebhookInstallOptions: envtest.WebhookInstallOptions{
Paths: []string{filepath.Join("..", "..", "config", "webhook")},
},
}

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())

err = v1.AddToScheme(scheme)
Expect(err).NotTo(HaveOccurred())

//+kubebuilder:scaffold:scheme

k8sClient, err = client.New(cfg, client.Options{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,
Host: webhookInstallOptions.LocalServingHost,
Port: webhookInstallOptions.LocalServingPort,
CertDir: webhookInstallOptions.LocalServingCertDir,
LeaderElection: false,
MetricsBindAddress: "0",
})
Expect(err).NotTo(HaveOccurred())

err = (&KubernetesImagePuller{}).SetupWebhookWithManager(mgr)
Expect(err).NotTo(HaveOccurred())

//+kubebuilder:scaffold:webhook

go func() {
err = mgr.Start(ctx)
if err != nil {
Expect(err).NotTo(HaveOccurred())
}
}()

// wait for the webhook server to get ready
dialer := &net.Dialer{Timeout: time.Second}
addrPort := fmt.Sprintf("%s:%d", webhookInstallOptions.LocalServingHost, webhookInstallOptions.LocalServingPort)
Eventually(func() error {
conn, err := tls.DialWithDialer(dialer, "tcp", addrPort, &tls.Config{InsecureSkipVerify: true})
if err != nil {
return err
}
conn.Close()
return nil
}).Should(Succeed())

}, 60)

var _ = Describe("Create KubernetesImagePuller resource", func() {

var kip *KubernetesImagePuller

BeforeEach(func() {
kip = &KubernetesImagePuller{
ObjectMeta: metav1.ObjectMeta{
Name: "test-kip",
Namespace: "default",
},
}
})

It("Should create a KubernetesImagePuller resource", func() {
Expect(k8sClient.Create(ctx, kip)).Should(Succeed())
Expect(k8sClient.Delete(ctx, kip)).Should(Succeed())
})

It("Should not be able to create a second KubernetesImagePuller resource in the same namespace", func() {
secondKip := kip.DeepCopy()
secondKip.ObjectMeta.Name = secondKip.ObjectMeta.Name + "-different"

Expect(k8sClient.Create(ctx, kip)).Should(Succeed())
err := k8sClient.Create(ctx, secondKip)
Expect(err.Error()).To(Equal("admission webhook \"vkubernetesimagepuller.kb.io\" denied the request: only one KubernetesImagePuller is allowed per namespace"))
Expect(k8sClient.Delete(ctx, kip)).Should(Succeed())
})

It("Should be able to create a second KubernetesImagePuller resource in a different namespace", func() {
secondKip := kip.DeepCopy()
secondKip.ObjectMeta.Name = secondKip.ObjectMeta.Name + "-different"
secondKip.ObjectMeta.Namespace = secondKip.ObjectMeta.Namespace + "-different"
newNamespace := &v1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: secondKip.ObjectMeta.Namespace,
},
}
Expect(k8sClient.Create(ctx, newNamespace)).Should(Succeed())
Expect(k8sClient.Create(ctx, kip)).Should(Succeed())
Expect(k8sClient.Create(ctx, secondKip)).Should(Succeed())
Expect(k8sClient.Delete(ctx, kip)).Should(Succeed())
Expect(k8sClient.Delete(ctx, secondKip)).Should(Succeed())
Expect(k8sClient.Delete(ctx, newNamespace)).Should(Succeed())
})
})

var _ = AfterSuite(func() {
cancel()
By("tearing down the test environment")
err := testEnv.Stop()
Expect(err).NotTo(HaveOccurred())
})
1 change: 0 additions & 1 deletion api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion build/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# https://access.redhat.com/containers/?tab=tags#/registry.access.redhat.com/ubi8/go-toolset
FROM registry.access.redhat.com/ubi8/go-toolset:1.20.10-3 as builder
ENV GOPATH=/go/
ARG SKIP_TESTS="false"
USER root

WORKDIR /workspace
Expand All @@ -32,7 +33,7 @@ COPY pkg/ pkg/
COPY hack/ hack/

# Test
RUN make test
RUN if [[ ${SKIP_TESTS} == "false" ]]; then make test; fi
# Build
RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o manager main.go

Expand Down
25 changes: 25 additions & 0 deletions config/certmanager/certificate.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# The following manifests contain a self-signed issuer CR and a certificate CR.
# More document can be found at https://docs.cert-manager.io
# WARNING: Targets CertManager v1.0. Check https://cert-manager.io/docs/installation/upgrading/ for breaking changes.
apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
name: selfsigned-issuer
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All resources names must be unique, like kubernetes-image-puller-selfsigned-issuer

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it should be prefixed by kustomize on generation

namespace: system
spec:
selfSigned: {}
---
apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
name: serving-cert # this name should match the one appeared in kustomizeconfig.yaml
namespace: system
spec:
# $(SERVICE_NAME) and $(SERVICE_NAMESPACE) will be substituted by kustomize
dnsNames:
- $(SERVICE_NAME).$(SERVICE_NAMESPACE).svc
- $(SERVICE_NAME).$(SERVICE_NAMESPACE).svc.cluster.local
issuerRef:
kind: Issuer
name: selfsigned-issuer
secretName: webhook-server-cert # this secret will not be prefixed, since it's not managed by kustomize
5 changes: 5 additions & 0 deletions config/certmanager/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
resources:
- certificate.yaml

configurations:
- kustomizeconfig.yaml
16 changes: 16 additions & 0 deletions config/certmanager/kustomizeconfig.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# This configuration is for teaching kustomize how to update name ref and var substitution
nameReference:
- kind: Issuer
group: cert-manager.io
fieldSpecs:
- kind: Certificate
group: cert-manager.io
path: spec/issuerRef/name

varReference:
- kind: Certificate
group: cert-manager.io
path: spec/commonName
- kind: Certificate
group: cert-manager.io
path: spec/dnsNames
Loading
Loading