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 17 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
2 changes: 1 addition & 1 deletion .github/workflows/pr-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,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
50 changes: 35 additions & 15 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@
# - use environment variables to overwrite this value (e.g export VERSION=0.0.2)
VERSION ?= 1.0.6

ifneq (,$(shell which kubectl 2>/dev/null)$(shell which oc 2>/dev/null))
include build/make/deploy.mk
endif

# Add silent flag for all commands by default
ifndef VERBOSE
MAKEFLAGS += --silent
endif

PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST))))
PROJECT_DIR := "$(shell pwd)"
CHECLUSTER_CRD_PATH = "$(PROJECT_DIR)/config/crd/bases/che.eclipse.org_kubernetesimagepullers.yaml"

# CHANNEL define the bundle package name
Expand All @@ -19,6 +23,9 @@ PACKAGE = kubernetes-imagepuller-operator
# CHANNEL define the bundle channel
CHANNEL = stable

# DEPLOYMENT_DIR defines the directory where the deployment manifests are generated
DEPLOYMENT_DIR=$(PROJECT_DIR)/deploy/deployment

# IMAGE_TAG_BASE defines the docker.io namespace and part of the image name for remote images.
# This variable is used to construct full image tags for bundle and catalog images.
IMAGE_TAG_BASE ?= quay.io/eclipse/kubernetes-image-puller-operator
Expand Down Expand Up @@ -94,13 +101,13 @@ run: manifests generate fmt vet ## Run a controller from your host.
##@ Development

docker-build: ## Build docker image with the manager.
docker build --no-cache -t ${IMG} -f build/Dockerfile .
$(IMAGE_TOOL) build --no-cache -t ${IMG} -f build/Dockerfile .

docker-push: ## Push docker image with the manager.
docker push ${IMG}
$(IMAGE_TOOL) push ${IMG}

manifests: download-controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
$(CONTROLLER_GEN) crd:crdVersions=v1 rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
$(CONTROLLER_GEN) crd:crdVersions=v1 rbac:roleName=manager-role paths="./..." output:crd:artifacts:config=config/crd/bases
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you remove webhook parameter, then you don't need annotation anymore // +kubebuilder:webhook:path ...


# remove yaml delimitier, which makes OLM catalog source image broken.
sed -i '/---/d' "$(CHECLUSTER_CRD_PATH)"
Expand All @@ -117,26 +124,38 @@ vet: ## Run go vet against code.
go vet ./...

ENVTEST_ASSETS_DIR=$(shell pwd)/testbin
test: SHELL := /bin/bash
test: manifests generate fmt vet ## Run tests.
mkdir -p ${ENVTEST_ASSETS_DIR}
test -f ${ENVTEST_ASSETS_DIR}/setup-envtest.sh || curl -sSLo ${ENVTEST_ASSETS_DIR}/setup-envtest.sh https://raw.githubusercontent.com/kubernetes-sigs/controller-runtime/v0.8.3/hack/setup-envtest.sh
source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out

install: manifests download-kustomize ## Install CRDs into the K8s cluster specified in ~/.kube/config.
$(KUSTOMIZE) build config/crd | kubectl apply -f -

uninstall: manifests download-kustomize ## Uninstall CRDs from the K8s cluster specified in ~/.kube/config.
$(KUSTOMIZE) build config/crd | kubectl delete -f -

deploy: manifests download-kustomize ## Deploy controller to the K8s cluster specified in ~/.kube/config.
# Set a new operator image for kustomize
kustomize-operator-image: download-kustomize
cd "$(PROJECT_DIR)/config/manager"
$(KUSTOMIZE) edit set image quay.io/eclipse/kubernetes-image-puller-operator:next=$(IMG)
cd "$(PROJECT_DIR)"

$(KUSTOMIZE) build config/default | kubectl apply -f -
gen-deployment: download-kustomize
rm -rf $(DEPLOYMENT_DIR)
for TARGET_PLATFORM in kubernetes openshift; do
PLATFORM_DIR=$(DEPLOYMENT_DIR)/$${TARGET_PLATFORM}
OBJECTS_DIR=$${PLATFORM_DIR}/objects

mkdir -p $${OBJECTS_DIR}

COMBINED_FILENAME=$${PLATFORM_DIR}/combined.yaml
$(KUSTOMIZE) build config/$${TARGET_PLATFORM} | cat > $${COMBINED_FILENAME} -

# Split the giant files output by kustomize per-object
csplit -s -f "temp" --suppress-matched "$${COMBINED_FILENAME}" '/^---$$/' '{*}'
for file in temp??; do
name_kind=$$(yq -r '"\(.metadata.name).\(.kind)"' "$${file}")
mv "$${file}" "$${OBJECTS_DIR}/$${name_kind}.yaml"
done
Copy link
Collaborator Author

Choose a reason for hiding this comment

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


undeploy: download-kustomize ## Undeploy controller from the K8s cluster specified in ~/.kube/config.
$(KUSTOMIZE) build config/default | kubectl delete -f -
echo "[INFO] Deployments resources generated into $${PLATFORM_DIR}"
done

compile:
binary="$(BINARY)"
Expand Down Expand Up @@ -169,7 +188,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.

4 changes: 3 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 @@ -30,9 +31,10 @@ COPY api/ api/
COPY controllers/ controllers/
COPY pkg/ pkg/
COPY hack/ hack/
COPY config/webhook/ config/webhook/

# 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
Loading
Loading