Skip to content

Commit

Permalink
feat: opt-in allowing cascade delete of namespaces
Browse files Browse the repository at this point in the history
  • Loading branch information
erikgb committed Nov 11, 2024
1 parent 960ef8f commit 008f558
Show file tree
Hide file tree
Showing 13 changed files with 298 additions and 22 deletions.
8 changes: 5 additions & 3 deletions .github/workflows/helm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,14 @@ jobs:
kubectl -n cert-manager wait --for=condition=available --timeout=180s --all deployments
- name: Prepare values.yaml
run: |
LATEST=$(curl -s "https://api.github.com/repos/cybozu-go/accurate/releases/latest" | jq -r .tag_name)
APP_VERSION=${LATEST#v}
docker build -t accurate:dev .
kind load docker-image accurate:dev --name=chart-testing
mkdir -p charts/accurate/ci/
cat > charts/accurate/ci/ci-values.yaml <<EOF
image:
tag: $APP_VERSION
repository: accurate
tag: dev
pullPolicy: Never
EOF
- name: Run chart-testing (install)
run: ct install --config ct.yaml
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ envtest: setup-envtest
source <($(SETUP_ENVTEST) use -p env); \
go test -v -count 1 -race ./controllers -ginkgo.progress -ginkgo.v -ginkgo.fail-fast
source <($(SETUP_ENVTEST) use -p env); \
go test -v -count 1 -race ./hooks -ginkgo.progress -ginkgo.v
go test -v -count 1 -race ./hooks/... -ginkgo.progress -ginkgo.v

.PHONY: test
test: test-tools
Expand Down
1 change: 1 addition & 0 deletions charts/accurate/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ $ helm install --create-namespace --namespace accurate accurate -f values.yaml a
| controller.replicas | int | `2` | Specify the number of replicas of the controller Pod. |
| controller.resources | object | `{"requests":{"cpu":"100m","memory":"20Mi"}}` | Specify resources. |
| controller.terminationGracePeriodSeconds | int | `10` | Specify terminationGracePeriodSeconds. |
| webhook.allowCascadingDeletion | bool | `false` | Enable to allow cascading deletion of namespaces. Accurate webhooks will only allow deletion of a namespace with children if this option is enabled. |
| image.pullPolicy | string | `nil` | Accurate image pullPolicy. |
| image.repository | string | `"ghcr.io/cybozu-go/accurate"` | Accurate image repository to use. |
| image.tag | string | `{{ .Chart.AppVersion }}` | Accurate image tag to use. |
Expand Down
3 changes: 2 additions & 1 deletion charts/accurate/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ spec:
{{- with .Values.image.pullPolicy }}
imagePullPolicy: {{ . }}
{{- end }}
{{- with .Values.controller.extraArgs }}
args:
- --webhook-allow-cascading-deletion={{ .Values.webhook.allowCascadingDeletion }}
{{- with .Values.controller.extraArgs }}
{{- toYaml . | nindent 12 }}
{{- end }}
ports:
Expand Down
9 changes: 9 additions & 0 deletions charts/accurate/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,12 @@ controller:
# common namespace-scoped resources.
clusterRoles:
- admin

webhook:
# webhook.allowCascadingDeletion -- Enable to allow cascading deletion of namespaces.
# Accurate webhooks will only allow deletion of a namespace with children if this option is enabled.
# Deleting namespaces is very dangerous, and deleting sub-namespaces can result in entire subtrees
# of namespaces being deleted as well. So enable this option with care!
# That said, enabling this option can be very useful to allow modern GitOps controllers like FluxCD
# to operate without errors based on desired state specified in Git.
allowCascadingDeletion: false
4 changes: 4 additions & 0 deletions cmd/accurate-controller/sub/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ var options struct {
certDir string
qps int
zapOpts zap.Options

webhookAllowCascadingDeletion bool
}

var rootCmd = &cobra.Command{
Expand Down Expand Up @@ -74,6 +76,8 @@ func init() {
fs.StringVar(&options.certDir, "cert-dir", "", "webhook certificate directory")
fs.IntVar(&options.qps, "apiserver-qps-throttle", defaultQPS, "The maximum QPS to the API server.")

fs.BoolVar(&options.webhookAllowCascadingDeletion, "webhook-allow-cascading-deletion", false, "Set to true to allow cascading deletion of namespaces (namespaces with children)")

config.DefaultMutableFeatureGate.AddFlag(fs)

goflags := flag.NewFlagSet("klog", flag.ExitOnError)
Expand Down
4 changes: 2 additions & 2 deletions cmd/accurate-controller/sub/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func subMain(ns, addr string, port int) error {
}).SetupWithManager(mgr); err != nil {
return fmt.Errorf("unable to create Namespace controller: %w", err)
}
hooks.SetupNamespaceWebhook(mgr, dec)
hooks.SetupNamespaceWebhook(mgr, dec, options.webhookAllowCascadingDeletion)

// SubNamespace reconciler & webhook
if err := indexing.SetupIndexForSubNamespace(ctx, mgr); err != nil {
Expand All @@ -141,7 +141,7 @@ func subMain(ns, addr string, port int) error {
}).SetupWithManager(mgr); err != nil {
return fmt.Errorf("unable to create SubNamespace controller: %w", err)
}
if err = hooks.SetupSubNamespaceWebhook(mgr, dec, cfg.NamingPolicyRegexps); err != nil {
if err = hooks.SetupSubNamespaceWebhook(mgr, dec, cfg.NamingPolicyRegexps, options.webhookAllowCascadingDeletion); err != nil {
return fmt.Errorf("unable to create SubNamespace webhook: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion docs/design.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ Since these are fundamentally different requirements, we decided to develop our
- Opt-in root namespaces
- Only namespaces labeled with `accurate.cybozu.com/type: root` can be the root of a namespace tree.
- Tenant users can create and delete sub-namespaces by creating and deleting a custom resource in a root or a sub-namespace.
- If a namespace has one or more sub-namespaces, Accurate prevents the deletion of the namespace.
- If a namespace has one or more sub-namespaces, Accurate prevents the deletion of the namespace - unless allow cascading deletion of namespaces is enabled.
- Template namespace
- Namespaces that are not a sub-namespace can specify a template from which labels, annotations, and resources can be propagated.
- Admins can change the parent namespace of a sub-namespace.
Expand Down
91 changes: 91 additions & 0 deletions hooks/allow-cascade-delete/cascade_delete_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package hooks_allow_cascade_delete

import (
"context"

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

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

accuratev2 "github.com/cybozu-go/accurate/api/accurate/v2"
"github.com/cybozu-go/accurate/pkg/constants"
)

var _ = Describe("Webhook allow cascade delete", func() {
var (
ctx context.Context
root *corev1.Namespace
)

BeforeEach(func() {
ctx = context.Background()

root = &corev1.Namespace{}
root.GenerateName = "cascade-root-"
root.Labels = map[string]string{constants.LabelType: constants.NSTypeRoot}
Expect(k8sClient.Create(ctx, root)).To(Succeed())
})

Context("Namespace", func() {
It("should ALLOW deleting a root with children", func() {
sub := &corev1.Namespace{}
sub.GenerateName = "cascade-sub-"
sub.Labels = map[string]string{constants.LabelParent: root.Name}
Expect(k8sClient.Create(ctx, sub)).To(Succeed())

Expect(k8sClient.Delete(ctx, root)).To(Succeed())
})

It("should ALLOW deleting a sub-namespace with children", func() {
sub := &corev1.Namespace{}
sub.GenerateName = "cascade-sub-"
sub.Labels = map[string]string{constants.LabelParent: root.Name}
Expect(k8sClient.Create(ctx, sub)).To(Succeed())

subSub := &corev1.Namespace{}
subSub.GenerateName = "cascade-sub-sub-"
subSub.Labels = map[string]string{constants.LabelParent: sub.Name}
Expect(k8sClient.Create(ctx, subSub)).To(Succeed())

Expect(k8sClient.Delete(ctx, sub)).To(Succeed())
})

It("should DENY deleting a template with children", func() {
tmpl := &corev1.Namespace{}
tmpl.GenerateName = "cascade-tmpl-"
tmpl.Labels = map[string]string{constants.LabelType: constants.NSTypeTemplate}
Expect(k8sClient.Create(ctx, tmpl)).To(Succeed())

root.Labels[constants.LabelTemplate] = tmpl.Name
Expect(k8sClient.Update(ctx, root)).To(Succeed())

err := k8sClient.Delete(ctx, tmpl)
Expect(err).To(HaveOccurred())
Expect(errors.ReasonForError(err)).Should(Equal(metav1.StatusReasonForbidden))
})
})

Context("SubNamespace", func() {
It("should ALLOW deletion with child namespaces", func() {
sub := &accuratev2.SubNamespace{}
sub.Namespace = root.Name
sub.GenerateName = "cascade-sub-"
Expect(k8sClient.Create(ctx, sub)).To(Succeed())
// Create sub-namespace since no controllers present in this test setup
subNS := &corev1.Namespace{}
subNS.Name = sub.Name
subNS.Labels = map[string]string{constants.LabelParent: root.Name}
Expect(k8sClient.Create(ctx, subNS)).To(Succeed())

ns := &corev1.Namespace{}
ns.GenerateName = "cascade-sub-sub-"
ns.Labels = map[string]string{constants.LabelParent: subNS.Name}
Expect(k8sClient.Create(ctx, ns)).To(Succeed())

Expect(k8sClient.Delete(ctx, sub)).To(Succeed())
})
})
})
160 changes: 160 additions & 0 deletions hooks/allow-cascade-delete/suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
package hooks_allow_cascade_delete

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

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

admissionv1beta1 "k8s.io/api/admission/v1beta1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
"sigs.k8s.io/controller-runtime/pkg/metrics/server"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
"sigs.k8s.io/kustomize/api/krusty"
"sigs.k8s.io/kustomize/kyaml/filesys"

accuratev1 "github.com/cybozu-go/accurate/api/accurate/v1"
accuratev2 "github.com/cybozu-go/accurate/api/accurate/v2"
accuratev2alpha1 "github.com/cybozu-go/accurate/api/accurate/v2alpha1"
"github.com/cybozu-go/accurate/hooks"
"github.com/cybozu-go/accurate/pkg/indexing"
)

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

var k8sClient client.Client
var testEnv *envtest.Environment
var cancelMgr context.CancelFunc

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

RunSpecs(t, "Webhook Suite")
}

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

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

scheme := runtime.NewScheme()
err := accuratev1.AddToScheme(scheme)
Expect(err).NotTo(HaveOccurred())
err = accuratev2alpha1.AddToScheme(scheme)
Expect(err).NotTo(HaveOccurred())
err = accuratev2.AddToScheme(scheme)
Expect(err).NotTo(HaveOccurred())
err = clientgoscheme.AddToScheme(scheme)
Expect(err).NotTo(HaveOccurred())
err = admissionv1beta1.AddToScheme(scheme)
Expect(err).NotTo(HaveOccurred())

//+kubebuilder:scaffold:scheme

By("bootstrapping test environment")
testEnv = &envtest.Environment{
Scheme: scheme,
CRDs: loadCRDs(),
WebhookInstallOptions: envtest.WebhookInstallOptions{
Paths: []string{filepath.Join("..", "..", "config", "webhook")},
},
}

cfg, err := testEnv.Start()
Expect(err).NotTo(HaveOccurred())
Expect(cfg).NotTo(BeNil())

k8sClient, err = client.New(cfg, client.Options{Scheme: scheme})
Expect(err).NotTo(HaveOccurred())
Expect(k8sClient).NotTo(BeNil())

webhookInstallOptions := &testEnv.WebhookInstallOptions
mgr, err := ctrl.NewManager(cfg, ctrl.Options{
Scheme: scheme,
WebhookServer: webhook.NewServer(webhook.Options{
Host: webhookInstallOptions.LocalServingHost,
Port: webhookInstallOptions.LocalServingPort,
CertDir: webhookInstallOptions.LocalServingCertDir,
}),
LeaderElection: false,
Metrics: server.Options{BindAddress: "0"},
})
Expect(err).NotTo(HaveOccurred())

err = indexing.SetupIndexForNamespace(ctx, mgr)
Expect(err).NotTo(HaveOccurred())

dec := admission.NewDecoder(scheme)
hooks.SetupNamespaceWebhook(mgr, dec, true)

Expect(err).NotTo(HaveOccurred())
err = hooks.SetupSubNamespaceWebhook(mgr, dec, nil, true)
Expect(err).NotTo(HaveOccurred())

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

})

var _ = AfterSuite(func() {
cancelMgr()
time.Sleep(50 * time.Millisecond)
By("tearing down the test environment")
err := testEnv.Stop()
Expect(err).NotTo(HaveOccurred())
})

func loadCRDs() []*apiextensionsv1.CustomResourceDefinition {
kOpts := krusty.MakeDefaultOptions()
k := krusty.MakeKustomizer(kOpts)
m, err := k.Run(filesys.FileSystemOrOnDisk{}, filepath.Join("..", "..", "config", "crd"))
Expect(err).To(Succeed())
resources := m.Resources()

crds := make([]*apiextensionsv1.CustomResourceDefinition, len(resources))
for i := range resources {
bytes, err := resources[i].MarshalJSON()
Expect(err).To(Succeed())

crd := &apiextensionsv1.CustomResourceDefinition{}
err = json.Unmarshal(bytes, crd)
Expect(err).To(Succeed())

crds[i] = crd
}

return crds
}
14 changes: 8 additions & 6 deletions hooks/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import (

type namespaceValidator struct {
client.Client
dec admission.Decoder
dec admission.Decoder
allowCascadingDeletion bool
}

var _ admission.Handler = &namespaceValidator{}
Expand Down Expand Up @@ -178,10 +179,10 @@ func (v *namespaceValidator) handleUpdate(ctx context.Context, nsNew, nsOld *cor
func (v *namespaceValidator) handleDelete(ctx context.Context, ns *corev1.Namespace) admission.Response {
key := constants.NamespaceParentKey
switch {
case ns.Labels[constants.LabelType] == constants.NSTypeRoot:
case ns.Labels[constants.LabelType] == constants.NSTypeRoot && !v.allowCascadingDeletion:
case ns.Labels[constants.LabelType] == constants.NSTypeTemplate:
key = constants.NamespaceTemplateKey
case ns.Labels[constants.LabelParent] != "":
case ns.Labels[constants.LabelParent] != "" && !v.allowCascadingDeletion:
default:
return admission.Allowed("")
}
Expand All @@ -198,10 +199,11 @@ func (v *namespaceValidator) handleDelete(ctx context.Context, ns *corev1.Namesp
}

// SetupNamespaceWebhook registers the webhook for Namespace
func SetupNamespaceWebhook(mgr manager.Manager, dec admission.Decoder) {
func SetupNamespaceWebhook(mgr manager.Manager, dec admission.Decoder, allowCascadingDeletion bool) {
v := &namespaceValidator{
Client: mgr.GetClient(),
dec: dec,
Client: mgr.GetClient(),
dec: dec,
allowCascadingDeletion: allowCascadingDeletion,
}
serv := mgr.GetWebhookServer()
serv.Register("/validate-v1-namespace", &webhook.Admission{Handler: v})
Expand Down
Loading

0 comments on commit 008f558

Please sign in to comment.