Skip to content

Commit fd9ccc5

Browse files
IvanHuntersIvanHunters
committed
feat(lineage-webhook): refuse a second ApplicationDefinition for a kind
A definition declaring an application kind that another definition already declares now fails when it is applied, instead of sitting next to the first one where it has no effect. An update that keeps its kind is allowed, so a duplicate that predates the check keeps upgrading, and so is a second definition from the same Helm release, since a release that renames its definition object creates the new one before it deletes the old one. The webhook is registered with failurePolicy Ignore: ApplicationDefinitions ship with the platform itself, so an outage of the webhook must never block a bootstrap or an upgrade, and a failure to read the existing definitions lets the request through with a warning rather than answering with an error, which Ignore would not cover. It is the feedback layer; which definition manages a kind is decided by the ownership rule whatever it lets through. Assisted-by: LLM Signed-off-by: IvanHunters <[email protected]>
1 parent c5c6d73 commit fd9ccc5

5 files changed

Lines changed: 283 additions & 0 deletions

File tree

‎cmd/lineage-controller-webhook/main.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
3535
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
3636
"sigs.k8s.io/controller-runtime/pkg/webhook"
37+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
3738

3839
cozystackiov1alpha1 "github.com/cozystack/cozystack/api/v1alpha1"
3940
lcw "github.com/cozystack/cozystack/internal/lineagecontrollerwebhook"
@@ -159,6 +160,9 @@ func main() {
159160
setupLog.Error(err, "unable to setup webhook", "webhook", "LineageWebhook")
160161
os.Exit(1)
161162
}
163+
mgr.GetWebhookServer().Register(lcw.ApplicationDefinitionKindValidatorPath, &admission.Webhook{
164+
Handler: lcw.NewApplicationDefinitionKindValidator(mgr.GetClient(), mgr.GetScheme()),
165+
})
162166

163167
// +kubebuilder:scaffold:builder
164168

‎internal/lineagecontrollerwebhook/appdef_validator.go‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package lineagecontrollerwebhook
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"net/http"
7+
8+
admissionv1 "k8s.io/api/admission/v1"
9+
"k8s.io/apimachinery/pkg/runtime"
10+
"sigs.k8s.io/controller-runtime/pkg/client"
11+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
12+
13+
cozyv1alpha1 "github.com/cozystack/cozystack/api/v1alpha1"
14+
"github.com/cozystack/cozystack/internal/shared/appdefowner"
15+
)
16+
17+
// ApplicationDefinitionKindValidatorPath is where the kind-uniqueness check is
18+
// served; the chart's ValidatingWebhookConfiguration points at it.
19+
const ApplicationDefinitionKindValidatorPath = "/validate-applicationdefinition"
20+
21+
// ApplicationDefinitionKindValidator refuses an ApplicationDefinition that
22+
// declares an application kind another definition already declares, so a
23+
// second definition for a kind fails loudly when it is applied instead of
24+
// sitting next to the first one. It is the feedback layer, not the guarantee:
25+
// the webhook is registered with failurePolicy Ignore, so an outage of the
26+
// webhook never blocks a platform install or upgrade, and appdefowner decides
27+
// which definition manages a kind whatever reaches the cluster.
28+
type ApplicationDefinitionKindValidator struct {
29+
reader client.Reader
30+
decoder admission.Decoder
31+
}
32+
33+
// NewApplicationDefinitionKindValidator builds the validator over reader,
34+
// normally the manager's cached client.
35+
func NewApplicationDefinitionKindValidator(reader client.Reader, scheme *runtime.Scheme) *ApplicationDefinitionKindValidator {
36+
return &ApplicationDefinitionKindValidator{reader: reader, decoder: admission.NewDecoder(scheme)}
37+
}
38+
39+
// Handle implements admission.Handler.
40+
func (v *ApplicationDefinitionKindValidator) Handle(ctx context.Context, req admission.Request) admission.Response {
41+
def := &cozyv1alpha1.ApplicationDefinition{}
42+
if err := v.decoder.DecodeRaw(req.Object, def); err != nil {
43+
return admission.Errored(http.StatusBadRequest, err)
44+
}
45+
kind := def.Spec.Application.Kind
46+
if req.Operation == admissionv1.Update {
47+
old := &cozyv1alpha1.ApplicationDefinition{}
48+
if err := v.decoder.DecodeRaw(req.OldObject, old); err != nil {
49+
return admission.Errored(http.StatusBadRequest, err)
50+
}
51+
// A definition that keeps its kind is not claiming anything new. This
52+
// also lets a duplicate that predates the check keep upgrading.
53+
if old.Spec.Application.Kind == kind {
54+
return admission.Allowed("")
55+
}
56+
}
57+
defs := &cozyv1alpha1.ApplicationDefinitionList{}
58+
if err := v.reader.List(ctx, defs); err != nil {
59+
// An error answer is a verdict that refuses the request, and
60+
// failurePolicy Ignore only covers a webhook that does not answer. The
61+
// check is feedback, not the guarantee, so let the request through and
62+
// say so.
63+
return admission.Allowed("").WithWarnings(fmt.Sprintf("application kind uniqueness was not checked: list ApplicationDefinitions: %v", err))
64+
}
65+
if kind == "" || !claimedByOther(defs.Items, def) {
66+
return admission.Allowed("")
67+
}
68+
return admission.Denied(fmt.Sprintf("application kind %s is already owned by ApplicationDefinition %s; delete that definition first to hand the kind over, or declare a different kind", kind, appdefowner.Owners(defs.Items)[kind]))
69+
}
70+
71+
// claimedByOther reports whether a definition other than def declares def's
72+
// kind. A definition from the same Helm release does not count: a release that
73+
// renames its definition object creates the new one before it deletes the old
74+
// one, and refusing that would fail the upgrade carrying the rename.
75+
func claimedByOther(defs []cozyv1alpha1.ApplicationDefinition, def *cozyv1alpha1.ApplicationDefinition) bool {
76+
for i := range defs {
77+
other := &defs[i]
78+
if other.Name == def.Name || other.Spec.Application.Kind != def.Spec.Application.Kind {
79+
continue
80+
}
81+
if sameHelmRelease(other, def) {
82+
continue
83+
}
84+
return true
85+
}
86+
return false
87+
}
88+
89+
func sameHelmRelease(a, b *cozyv1alpha1.ApplicationDefinition) bool {
90+
const name, namespace = "meta.helm.sh/release-name", "meta.helm.sh/release-namespace"
91+
an := a.GetAnnotations()[name]
92+
return an != "" && an == b.GetAnnotations()[name] &&
93+
a.GetAnnotations()[namespace] == b.GetAnnotations()[namespace]
94+
}

‎internal/lineagecontrollerwebhook/appdef_validator_test.go‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
package lineagecontrollerwebhook
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"strings"
7+
"testing"
8+
9+
admissionv1 "k8s.io/api/admission/v1"
10+
"k8s.io/apimachinery/pkg/api/errors"
11+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
"k8s.io/apimachinery/pkg/runtime"
13+
"sigs.k8s.io/controller-runtime/pkg/client"
14+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
15+
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
16+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
17+
18+
cozyv1alpha1 "github.com/cozystack/cozystack/api/v1alpha1"
19+
)
20+
21+
func appDef(name, kind string) *cozyv1alpha1.ApplicationDefinition {
22+
d := &cozyv1alpha1.ApplicationDefinition{
23+
TypeMeta: metav1.TypeMeta{APIVersion: "cozystack.io/v1alpha1", Kind: "ApplicationDefinition"},
24+
ObjectMeta: metav1.ObjectMeta{Name: name},
25+
}
26+
d.Spec.Application.Kind = kind
27+
return d
28+
}
29+
30+
func admissionReq(t *testing.T, op admissionv1.Operation, obj, old *cozyv1alpha1.ApplicationDefinition) admission.Request {
31+
t.Helper()
32+
raw, err := json.Marshal(obj)
33+
if err != nil {
34+
t.Fatal(err)
35+
}
36+
req := admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: op, Name: obj.Name, Object: runtime.RawExtension{Raw: raw}}}
37+
if old != nil {
38+
oldRaw, err := json.Marshal(old)
39+
if err != nil {
40+
t.Fatal(err)
41+
}
42+
req.OldObject = runtime.RawExtension{Raw: oldRaw}
43+
}
44+
return req
45+
}
46+
47+
func TestAppDefKindValidator(t *testing.T) {
48+
scheme := newWebhookScheme(t)
49+
existing := appDef("redis", "Redis")
50+
cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existing, appDef("shadow", "Redis")).Build()
51+
v := NewApplicationDefinitionKindValidator(cl, scheme)
52+
53+
cases := []struct {
54+
name string
55+
req admission.Request
56+
allowed bool
57+
}{
58+
{"create with a kind another definition owns is refused", admissionReq(t, admissionv1.Create, appDef("tap-redis", "Redis"), nil), false},
59+
{"create with a free kind is allowed", admissionReq(t, admissionv1.Create, appDef("pg", "Postgres"), nil), true},
60+
{"update of the owner itself is allowed", admissionReq(t, admissionv1.Update, appDef("redis", "Redis"), appDef("redis", "Redis")), true},
61+
// A duplicate that predates the check keeps upgrading; ownership, not
62+
// admission, keeps it from managing the kind's releases.
63+
{"update of a pre-existing duplicate that keeps its kind is allowed", admissionReq(t, admissionv1.Update, appDef("shadow", "Redis"), appDef("shadow", "Redis")), true},
64+
{"update switching to a claimed kind is refused", admissionReq(t, admissionv1.Update, appDef("pg", "Redis"), appDef("pg", "Postgres")), false},
65+
}
66+
for _, tc := range cases {
67+
t.Run(tc.name, func(t *testing.T) {
68+
resp := v.Handle(context.TODO(), tc.req)
69+
if resp.Allowed != tc.allowed {
70+
t.Fatalf("allowed = %v, want %v (%v)", resp.Allowed, tc.allowed, resp.Result)
71+
}
72+
})
73+
}
74+
}
75+
76+
func helmOwned(d *cozyv1alpha1.ApplicationDefinition, release string) *cozyv1alpha1.ApplicationDefinition {
77+
d.Annotations = map[string]string{"meta.helm.sh/release-name": release, "meta.helm.sh/release-namespace": "cozy-system"}
78+
return d
79+
}
80+
81+
// A Helm release that renames its definition object creates the new one before
82+
// it deletes the old one, so both briefly declare the kind. Refusing that would
83+
// fail the platform upgrade that carries the rename.
84+
func TestAppDefKindValidatorAllowsRenameWithinOneHelmRelease(t *testing.T) {
85+
scheme := newWebhookScheme(t)
86+
cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(helmOwned(appDef("redis", "Redis"), "redis-rd")).Build()
87+
v := NewApplicationDefinitionKindValidator(cl, scheme)
88+
89+
if resp := v.Handle(context.TODO(), admissionReq(t, admissionv1.Create, helmOwned(appDef("redis-v2", "Redis"), "redis-rd"), nil)); !resp.Allowed {
90+
t.Fatalf("rename within one Helm release refused: %v", resp.Result)
91+
}
92+
resp := v.Handle(context.TODO(), admissionReq(t, admissionv1.Create, helmOwned(appDef("tap-redis", "Redis"), "tap-redis-rd"), nil))
93+
if resp.Allowed {
94+
t.Fatal("a definition from another Helm release must still be refused")
95+
}
96+
otherNS := helmOwned(appDef("redis-elsewhere", "Redis"), "redis-rd")
97+
otherNS.Annotations["meta.helm.sh/release-namespace"] = "tenant-foo"
98+
if resp := v.Handle(context.TODO(), admissionReq(t, admissionv1.Create, otherNS, nil)); resp.Allowed {
99+
t.Fatal("a release of the same name in another namespace is a different release and must be refused")
100+
}
101+
if !strings.Contains(resp.Result.Message, "redis") {
102+
t.Fatalf("the refusal must name the definition that declares the kind, got %q", resp.Result.Message)
103+
}
104+
}
105+
106+
// The check is feedback, not the guarantee, so failing to read the existing
107+
// definitions must let the request through rather than refuse it: a 500 answer
108+
// is a verdict, which failurePolicy Ignore does not cover.
109+
func TestAppDefKindValidatorFailsOpenOnListError(t *testing.T) {
110+
scheme := newWebhookScheme(t)
111+
cl := interceptor.NewClient(fake.NewClientBuilder().WithScheme(scheme).Build(), interceptor.Funcs{
112+
List: func(context.Context, client.WithWatch, client.ObjectList, ...client.ListOption) error {
113+
return errors.NewServiceUnavailable("cache not started")
114+
},
115+
})
116+
v := NewApplicationDefinitionKindValidator(cl, scheme)
117+
resp := v.Handle(context.TODO(), admissionReq(t, admissionv1.Create, appDef("tap-redis", "Redis"), nil))
118+
if !resp.Allowed {
119+
t.Fatalf("a failed read must not refuse the request: %v", resp.Result)
120+
}
121+
if len(resp.Warnings) == 0 {
122+
t.Fatal("letting a request through unchecked must say so in a warning")
123+
}
124+
}

‎packages/system/lineage-controller-webhook/templates/validatingwebhookconfiguration.yaml‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
apiVersion: admissionregistration.k8s.io/v1
2+
kind: ValidatingWebhookConfiguration
3+
metadata:
4+
name: lineage-applicationdefinition-kind
5+
annotations:
6+
cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/lineage-controller-webhook
7+
labels:
8+
app: cozystack-controller
9+
webhooks:
10+
# Refuses a second ApplicationDefinition for an application kind that is
11+
# already declared. failurePolicy Ignore: ApplicationDefinitions ship with the
12+
# platform itself, so a webhook that is not up yet must not block a bootstrap
13+
# or an upgrade. Which definition manages a kind is decided in code
14+
# (internal/shared/appdefowner) whatever this check lets through.
15+
- name: applicationdefinition-kind.cozystack.io
16+
admissionReviewVersions: ["v1"]
17+
sideEffects: None
18+
timeoutSeconds: 5
19+
clientConfig:
20+
service:
21+
name: lineage-controller-webhook
22+
namespace: {{ .Release.Namespace }}
23+
path: /validate-applicationdefinition
24+
rules:
25+
- operations: ["CREATE", "UPDATE"]
26+
apiGroups: ["cozystack.io"]
27+
apiVersions: ["v1alpha1"]
28+
resources: ["applicationdefinitions"]
29+
scope: Cluster
30+
failurePolicy: Ignore

‎packages/system/lineage-controller-webhook/tests/validatingwebhookconfiguration_test.yaml‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
suite: lineage-controller-webhook ApplicationDefinition kind check
2+
templates:
3+
- templates/validatingwebhookconfiguration.yaml
4+
5+
release:
6+
name: lineage-controller-webhook
7+
namespace: cozy-system
8+
9+
tests:
10+
- it: routes ApplicationDefinition create and update to the kind check without blocking on an outage
11+
asserts:
12+
- isKind:
13+
of: ValidatingWebhookConfiguration
14+
- equal:
15+
path: webhooks[0].clientConfig.service.path
16+
value: /validate-applicationdefinition
17+
- equal:
18+
path: webhooks[0].clientConfig.service.namespace
19+
value: cozy-system
20+
- equal:
21+
path: webhooks[0].failurePolicy
22+
value: Ignore
23+
- equal:
24+
path: webhooks[0].rules[0].resources
25+
value: ["applicationdefinitions"]
26+
- equal:
27+
path: webhooks[0].rules[0].operations
28+
value: ["CREATE", "UPDATE"]
29+
- equal:
30+
path: metadata.annotations["cert-manager.io/inject-ca-from"]
31+
value: cozy-system/lineage-controller-webhook

0 commit comments

Comments
 (0)