Skip to content

Commit d81aef5

Browse files
dkoshkinjimmidyson
dkoshkin
authored and
jimmidyson
committed
refactor: how handlers are added to server
When adding new handlers I kept forgetting to add it to the meta hander. Although this commit doesn't really solve it does make it easier to visually see a difference. It also simplifies adding handlers by not asking for a package variable when calling <patchpackage>.NewPatch.
1 parent 4f7e62e commit d81aef5

File tree

21 files changed

+122
-70
lines changed

21 files changed

+122
-70
lines changed

‎cmd/main.go

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"sigs.k8s.io/controller-runtime/pkg/manager"
2525
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
2626

27+
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/clustertopology/handlers"
2728
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/clustertopology/handlers/mutation"
2829
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/server"
2930
"github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/auditpolicy"
@@ -112,43 +113,52 @@ func main() {
112113
os.Exit(1)
113114
}
114115

115-
runtimeWebhookServer := server.NewServer(
116-
runtimeWebhookServerOpts,
117-
116+
// Handlers for lifecycle hooks.
117+
lifeCycleHandlers := []handlers.Named{
118118
servicelbgc.New(mgr.GetClient()),
119-
120-
calico.New(mgr.GetClient(), calicoCNIConfig, clusterconfig.VariableName, "cni"),
121-
119+
}
120+
// Handlers that apply patches to the Cluster object and its objects.
121+
// Used by CAPI's GeneratePatches hook.
122+
patchHandlers := []handlers.Named{
123+
httpproxy.NewPatch(mgr.GetClient()),
124+
extraapiservercertsans.NewPatch(),
125+
auditpolicy.NewPatch(),
126+
kubernetesimagerepository.NewPatch(),
127+
etcd.NewPatch(),
128+
}
129+
// Handlers used by CAPI's DiscoverVariables hook.
130+
// It's ok that this does not match patchHandlers.
131+
// Some of those handlers may always get applied and not have a corresponding variable.
132+
variableHandlers := []handlers.Named{
122133
httpproxy.NewVariable(),
123-
httpproxy.NewPatch(mgr.GetClient(), httpproxy.VariableName),
124-
125134
extraapiservercertsans.NewVariable(),
126-
extraapiservercertsans.NewPatch(extraapiservercertsans.VariableName),
127-
135+
kubernetesimagerepository.NewVariable(),
136+
}
137+
// This metaPatchHandlers combines all other patch and variable handlers under a single handler.
138+
// It allows to specify configuration under a single variable.
139+
metaPatchHandlers := []mutation.GeneratePatches{
140+
httpproxy.NewMetaPatch(mgr.GetClient()),
141+
extraapiservercertsans.NewMetaPatch(),
128142
auditpolicy.NewPatch(),
143+
kubernetesimagerepository.NewMetaPatch(),
144+
etcd.NewMetaPatch(),
145+
}
146+
metaHandlers := []handlers.Named{
147+
// This Calico handler relies on a variable but does not generate a patch.
148+
// Instead it creates other resources in the API.
149+
calico.NewMetaHandler(mgr.GetClient(), calicoCNIConfig),
150+
clusterconfig.NewVariable(),
151+
mutation.NewMetaGeneratePatchesHandler("clusterConfigPatch", metaPatchHandlers...),
152+
}
129153

130-
kubernetesimagerepository.NewVariable(),
131-
kubernetesimagerepository.NewPatch(kubernetesimagerepository.VariableName),
154+
var allHandlers []handlers.Named
155+
allHandlers = append(allHandlers, lifeCycleHandlers...)
156+
allHandlers = append(allHandlers, patchHandlers...)
157+
allHandlers = append(allHandlers, variableHandlers...)
158+
allHandlers = append(allHandlers, metaHandlers...)
132159

133-
etcd.NewVariable(),
134-
etcd.NewPatch(etcd.VariableName),
160+
runtimeWebhookServer := server.NewServer(runtimeWebhookServerOpts, allHandlers...)
135161

136-
clusterconfig.NewVariable(),
137-
mutation.NewMetaGeneratePatchesHandler(
138-
"clusterConfigPatch",
139-
httpproxy.NewPatch(mgr.GetClient(), clusterconfig.VariableName, httpproxy.VariableName),
140-
extraapiservercertsans.NewPatch(
141-
clusterconfig.VariableName,
142-
extraapiservercertsans.VariableName,
143-
),
144-
auditpolicy.NewPatch(),
145-
kubernetesimagerepository.NewPatch(
146-
clusterconfig.VariableName,
147-
kubernetesimagerepository.VariableName,
148-
),
149-
etcd.NewPatch(clusterconfig.VariableName, etcd.VariableName),
150-
),
151-
)
152162
if err := mgr.Add(runtimeWebhookServer); err != nil {
153163
setupLog.Error(err, "unable to add runtime webhook server runnable to controller manager")
154164
os.Exit(1)

‎common/pkg/capi/clustertopology/handlers/mutation/meta.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ import (
1212
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/clustertopology/handlers"
1313
)
1414

15+
const (
16+
// MetaVariableName is the meta cluster config patch variable name.
17+
MetaVariableName = "clusterConfig"
18+
)
19+
1520
type metaGeneratePatches struct {
1621
name string
1722
wrappedHandlers []GeneratePatches

‎pkg/handlers/clusterconfig/variables.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,6 @@ var (
2020
)
2121

2222
const (
23-
// VariableName is http proxy external patch variable name.
24-
VariableName = "clusterConfig"
25-
2623
// HandlerNameVariable is the name of the variable handler.
2724
HandlerNameVariable = "ClusterConfigVars"
2825
)
@@ -43,7 +40,7 @@ func (h *clusterConfigVariableHandler) DiscoverVariables(
4340
resp *runtimehooksv1.DiscoverVariablesResponse,
4441
) {
4542
resp.Variables = append(resp.Variables, clusterv1.ClusterClassVariable{
46-
Name: VariableName,
43+
Name: mutation.MetaVariableName,
4744
Required: false,
4845
Schema: v1alpha1.ClusterConfigSpec{}.VariableSchema(),
4946
})

‎pkg/handlers/clusterconfig/variables_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,14 @@ import (
99
"k8s.io/utils/ptr"
1010

1111
"github.com/d2iq-labs/capi-runtime-extensions/api/v1alpha1"
12+
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/clustertopology/handlers/mutation"
1213
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/testutils/capitest"
1314
)
1415

1516
func TestVariableValidation(t *testing.T) {
1617
capitest.ValidateDiscoverVariables(
1718
t,
18-
VariableName,
19+
mutation.MetaVariableName,
1920
ptr.To(v1alpha1.ClusterConfigSpec{}.VariableSchema()),
2021
NewVariable,
2122
capitest.VariableTestDef{

‎pkg/handlers/cni/calico/handler.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,17 @@ import (
2626
"github.com/d2iq-labs/capi-runtime-extensions/api/v1alpha1"
2727
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/clustertopology/handlers"
2828
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/clustertopology/handlers/lifecycle"
29+
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/clustertopology/handlers/mutation"
2930
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/clustertopology/variables"
3031
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/k8s/client"
3132
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/k8s/parser"
3233
"github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/cni"
3334
)
3435

36+
const (
37+
variableName = "cni"
38+
)
39+
3540
type CalicoCNIConfig struct {
3641
defaultsNamespace string
3742

@@ -78,17 +83,15 @@ var (
7883
calicoInstallationGK = schema.GroupKind{Group: "operator.tigera.io", Kind: "Installation"}
7984
)
8085

81-
func New(
86+
func NewMetaHandler(
8287
c ctrlclient.Client,
8388
cfg *CalicoCNIConfig,
84-
variableName string,
85-
variablePath ...string,
8689
) *CalicoCNI {
8790
return &CalicoCNI{
8891
client: c,
8992
config: cfg,
90-
variableName: variableName,
91-
variablePath: variablePath,
93+
variableName: mutation.MetaVariableName,
94+
variablePath: []string{variableName},
9295
}
9396
}
9497

‎pkg/handlers/etcd/inject.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,15 @@ var (
4141
_ mutation.GeneratePatches = &etcdPatchHandler{}
4242
)
4343

44-
func NewPatch(
44+
func NewPatch() *etcdPatchHandler {
45+
return newEtcdPatchHandler(variableName)
46+
}
47+
48+
func NewMetaPatch() *etcdPatchHandler {
49+
return newEtcdPatchHandler(mutation.MetaVariableName, variableName)
50+
}
51+
52+
func newEtcdPatchHandler(
4553
variableName string,
4654
variableFieldPath ...string,
4755
) *etcdPatchHandler {

‎pkg/handlers/etcd/inject_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@ import (
1717
func TestGeneratePatches(t *testing.T) {
1818
capitest.ValidateGeneratePatches(
1919
t,
20-
func() mutation.GeneratePatches { return NewPatch(VariableName) },
20+
func() mutation.GeneratePatches { return NewPatch() },
2121
capitest.PatchTestDef{
2222
Name: "unset variable",
2323
},
2424
capitest.PatchTestDef{
2525
Name: "etcd imageRepository and imageTag set",
2626
Vars: []runtimehooksv1.Variable{
2727
capitest.VariableWithValue(
28-
VariableName,
28+
variableName,
2929
v1alpha1.Etcd{
3030
Image: &v1alpha1.Image{
3131
Repository: "my-registry.io/my-org/my-repo",
@@ -55,7 +55,7 @@ func TestGeneratePatches(t *testing.T) {
5555
Name: "etcd imageRepository set",
5656
Vars: []runtimehooksv1.Variable{
5757
capitest.VariableWithValue(
58-
VariableName,
58+
variableName,
5959
v1alpha1.Etcd{
6060
Image: &v1alpha1.Image{
6161
Repository: "my-registry.io/my-org/my-repo",
@@ -83,7 +83,7 @@ func TestGeneratePatches(t *testing.T) {
8383
Name: "etcd imageTag set",
8484
Vars: []runtimehooksv1.Variable{
8585
capitest.VariableWithValue(
86-
VariableName,
86+
variableName,
8787
v1alpha1.Etcd{
8888
Image: &v1alpha1.Image{
8989
Tag: "v3.5.99_custom.0",

‎pkg/handlers/etcd/variables.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ var (
2020
)
2121

2222
const (
23-
// VariableName is http proxy external patch variable name.
24-
VariableName = "etcd"
23+
// variableName is etcd external patch variable name.
24+
variableName = "etcd"
2525

2626
// HandlerNameVariable is the name of the variable handler.
2727
HandlerNameVariable = "etcdVars"
@@ -43,7 +43,7 @@ func (h *etcdVariableHandler) DiscoverVariables(
4343
resp *runtimehooksv1.DiscoverVariablesResponse,
4444
) {
4545
resp.Variables = append(resp.Variables, clusterv1.ClusterClassVariable{
46-
Name: VariableName,
46+
Name: variableName,
4747
Required: false,
4848
Schema: v1alpha1.Etcd{}.VariableSchema(),
4949
})

‎pkg/handlers/etcd/variables_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
func TestVariableValidation(t *testing.T) {
1616
capitest.ValidateDiscoverVariables(
1717
t,
18-
VariableName,
18+
variableName,
1919
ptr.To(v1alpha1.Etcd{}.VariableSchema()),
2020
NewVariable,
2121
capitest.VariableTestDef{

‎pkg/handlers/extraapiservercertsans/inject.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,15 @@ var (
4141
_ mutation.GeneratePatches = &extraAPIServerCertSANsPatchHandler{}
4242
)
4343

44-
func NewPatch(
44+
func NewPatch() *extraAPIServerCertSANsPatchHandler {
45+
return newExtraAPIServerCertSANsPatchHandler(variableName)
46+
}
47+
48+
func NewMetaPatch() *extraAPIServerCertSANsPatchHandler {
49+
return newExtraAPIServerCertSANsPatchHandler(mutation.MetaVariableName, variableName)
50+
}
51+
52+
func newExtraAPIServerCertSANsPatchHandler(
4553
variableName string,
4654
variableFieldPath ...string,
4755
) *extraAPIServerCertSANsPatchHandler {

‎pkg/handlers/extraapiservercertsans/inject_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@ import (
1717
func TestGeneratePatches(t *testing.T) {
1818
capitest.ValidateGeneratePatches(
1919
t,
20-
func() mutation.GeneratePatches { return NewPatch(VariableName) },
20+
func() mutation.GeneratePatches { return NewPatch() },
2121
capitest.PatchTestDef{
2222
Name: "unset variable",
2323
},
2424
capitest.PatchTestDef{
2525
Name: "extra API server cert SANs set",
2626
Vars: []runtimehooksv1.Variable{
2727
capitest.VariableWithValue(
28-
VariableName,
28+
variableName,
2929
v1alpha1.ExtraAPIServerCertSANs{"a.b.c.example.com", "d.e.f.example.com"},
3030
),
3131
},

‎pkg/handlers/extraapiservercertsans/variables.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ var (
2020
)
2121

2222
const (
23-
// VariableName is http proxy external patch variable name.
24-
VariableName = "extraAPIServerCertSANs"
23+
// variableName is http proxy external patch variable name.
24+
variableName = "extraAPIServerCertSANs"
2525

2626
// HandlerNameVariable is the name of the variable handler.
2727
HandlerNameVariable = "ExtraAPIServerCertSANsVars"
@@ -43,7 +43,7 @@ func (h *extraAPIServerCertSANsVariableHandler) DiscoverVariables(
4343
resp *runtimehooksv1.DiscoverVariablesResponse,
4444
) {
4545
resp.Variables = append(resp.Variables, clusterv1.ClusterClassVariable{
46-
Name: VariableName,
46+
Name: variableName,
4747
Required: false,
4848
Schema: v1alpha1.ExtraAPIServerCertSANs{}.VariableSchema(),
4949
})

‎pkg/handlers/extraapiservercertsans/variables_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
func TestVariableValidation(t *testing.T) {
1616
capitest.ValidateDiscoverVariables(
1717
t,
18-
VariableName,
18+
variableName,
1919
ptr.To(v1alpha1.ExtraAPIServerCertSANs{}.VariableSchema()),
2020
NewVariable,
2121
capitest.VariableTestDef{

‎pkg/handlers/httpproxy/inject.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,18 @@ var (
5353

5454
func NewPatch(
5555
cl ctrlclient.Reader,
56+
) *httpProxyPatchHandler {
57+
return newHTTPProxyPatchHandler(cl, variableName)
58+
}
59+
60+
func NewMetaPatch(
61+
cl ctrlclient.Reader,
62+
) *httpProxyPatchHandler {
63+
return newHTTPProxyPatchHandler(cl, mutation.MetaVariableName, variableName)
64+
}
65+
66+
func newHTTPProxyPatchHandler(
67+
cl ctrlclient.Reader,
5668
variableName string,
5769
variableFieldPath ...string,
5870
) *httpProxyPatchHandler {

‎pkg/handlers/httpproxy/inject_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func TestGeneratePatches(t *testing.T) {
2222
t,
2323
func() *httpProxyPatchHandler {
2424
fakeClient := fake.NewClientBuilder().Build()
25-
return NewPatch(fakeClient, VariableName)
25+
return NewPatch(fakeClient)
2626
},
2727
capitest.PatchTestDef{
2828
Name: "unset variable",
@@ -31,7 +31,7 @@ func TestGeneratePatches(t *testing.T) {
3131
Name: "http proxy set for KubeadmConfigTemplate default-worker",
3232
Vars: []runtimehooksv1.Variable{
3333
capitest.VariableWithValue(
34-
VariableName,
34+
variableName,
3535
v1alpha1.HTTPProxy{
3636
HTTP: "http://example.com",
3737
HTTPS: "https://example.com",
@@ -58,7 +58,7 @@ func TestGeneratePatches(t *testing.T) {
5858
Name: "http proxy set for KubeadmConfigTemplate generic worker",
5959
Vars: []runtimehooksv1.Variable{
6060
capitest.VariableWithValue(
61-
VariableName,
61+
variableName,
6262
v1alpha1.HTTPProxy{
6363
HTTP: "http://example.com",
6464
HTTPS: "https://example.com",
@@ -85,7 +85,7 @@ func TestGeneratePatches(t *testing.T) {
8585
Name: "http proxy set for KubeadmControlPlaneTemplate",
8686
Vars: []runtimehooksv1.Variable{
8787
capitest.VariableWithValue(
88-
VariableName,
88+
variableName,
8989
v1alpha1.HTTPProxy{
9090
HTTP: "http://example.com",
9191
HTTPS: "https://example.com",

‎pkg/handlers/httpproxy/variables.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ var (
2020
)
2121

2222
const (
23-
// VariableName is http proxy external patch variable name.
24-
VariableName = "proxy"
23+
// variableName is http proxy external patch variable name.
24+
variableName = "proxy"
2525

2626
// HandlerNameVariable is the name of the variable handler.
2727
HandlerNameVariable = "HTTPProxyVars"
@@ -43,7 +43,7 @@ func (h *httpProxyVariableHandler) DiscoverVariables(
4343
resp *runtimehooksv1.DiscoverVariablesResponse,
4444
) {
4545
resp.Variables = append(resp.Variables, clusterv1.ClusterClassVariable{
46-
Name: VariableName,
46+
Name: variableName,
4747
Required: false,
4848
Schema: v1alpha1.HTTPProxy{}.VariableSchema(),
4949
})

0 commit comments

Comments
 (0)