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

refactor: stop using inject framework in controller-runtime #958

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion config/tests/samples/create/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,8 @@ func NewHarness(t *testing.T, ctx context.Context) *Harness {
if len(webhooks) > 0 {
server := mgr.GetWebhookServer()
for _, cfg := range webhooks {
server.Register(cfg.Path, &webhook.Admission{Handler: cfg.Handler})
handler := cfg.HandlerFunc(mgr)
server.Register(cfg.Path, &webhook.Admission{Handler: handler})
}
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/test/controller/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ func startTestManager(env *envtest.Environment, testType test.TestType, whCfgs [
if testType == test.IntegrationTestType {
server := mgr.GetWebhookServer()
for _, cfg := range whCfgs {
server.Register(cfg.Path, &webhook.Admission{Handler: cfg.Handler})
handler := cfg.HandlerFunc(mgr)
server.Register(cfg.Path, &webhook.Admission{Handler: handler})
}
}
stop := startMgr(mgr, log.Fatalf)
Expand Down
7 changes: 7 additions & 0 deletions pkg/webhook/abandon_on_uninstall_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package webhook
import (
"context"

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

Expand All @@ -25,6 +26,12 @@ import (
// attempted to be deleted.
type abandonOnCRDUninstallWebhook struct{}

func NewAbandonOnCRDUninstallWebhook() HandlerFunc {
return func(mgr manager.Manager) admission.Handler {
return &abandonOnCRDUninstallWebhook{}
}
}

// This webhook is now a no-op and will soon be removed as deletiondefender does not need this layer of protection any
// longer. The reason to keep it for now is that the operator does not yet remove the old webhook registration. The
// operator will be updated to remove this webhook registration and then the code can be deleted.
Expand Down
24 changes: 9 additions & 15 deletions pkg/webhook/container_annotation_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (
apimachinerytypes "k8s.io/apimachinery/pkg/types"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

Expand All @@ -44,23 +44,17 @@ type containerAnnotationHandler struct {
smLoader *servicemappingloader.ServiceMappingLoader
}

func NewContainerAnnotationHandler(smLoader *servicemappingloader.ServiceMappingLoader, dclSchemaLoader dclschemaloader.DCLSchemaLoader, serviceMetadataLoader dclmetadata.ServiceMetadataLoader) *containerAnnotationHandler {
return &containerAnnotationHandler{
smLoader: smLoader,
serviceMetadataLoader: serviceMetadataLoader,
dclSchemaLoader: dclSchemaLoader,
func NewContainerAnnotationHandler(smLoader *servicemappingloader.ServiceMappingLoader, dclSchemaLoader dclschemaloader.DCLSchemaLoader, serviceMetadataLoader dclmetadata.ServiceMetadataLoader) HandlerFunc {
return func(mgr manager.Manager) admission.Handler {
return &containerAnnotationHandler{
client: mgr.GetClient(),
smLoader: smLoader,
serviceMetadataLoader: serviceMetadataLoader,
dclSchemaLoader: dclSchemaLoader,
}
}
}

// containerAnnotationHandler implements inject.Client.
var _ inject.Client = &containerAnnotationHandler{}

// InjectClient injects the client into the containerAnnotationHandler
func (a *containerAnnotationHandler) InjectClient(c client.Client) error {
a.client = c
return nil
}

func (a *containerAnnotationHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
deserializer := codecs.UniversalDeserializer()
obj := &unstructured.Unstructured{}
Expand Down
7 changes: 5 additions & 2 deletions pkg/webhook/generic_defaulter.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

Expand All @@ -36,8 +37,10 @@ import (
type genericDefaulter struct {
}

func NewGenericDefaulter() *genericDefaulter {
return &genericDefaulter{}
func NewGenericDefaulter() HandlerFunc {
return func(mgr manager.Manager) admission.Handler {
return &genericDefaulter{}
}
}

func (a *genericDefaulter) Handle(ctx context.Context, req admission.Request) admission.Response {
Expand Down
11 changes: 7 additions & 4 deletions pkg/webhook/iam_defaulter.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

Expand All @@ -40,10 +41,12 @@ type iamDefaulter struct {
}

func NewIAMDefaulter(smLoader *servicemappingloader.ServiceMappingLoader,
serviceMetadataLoader metadata.ServiceMetadataLoader) *iamDefaulter {
return &iamDefaulter{
smLoader: smLoader,
serviceMetadataLoader: serviceMetadataLoader,
serviceMetadataLoader metadata.ServiceMetadataLoader) HandlerFunc {
return func(mgr manager.Manager) admission.Handler {
return &iamDefaulter{
smLoader: smLoader,
serviceMetadataLoader: serviceMetadataLoader,
}
}
}

Expand Down
13 changes: 8 additions & 5 deletions pkg/webhook/iam_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

Expand All @@ -44,11 +45,13 @@ type iamValidatorHandler struct {

func NewIAMValidatorHandler(smLoader *servicemappingloader.ServiceMappingLoader,
serviceMetadataLoader metadata.ServiceMetadataLoader,
schemaLoader dclschemaloader.DCLSchemaLoader) *iamValidatorHandler {
return &iamValidatorHandler{
smLoader: smLoader,
serviceMetadataLoader: serviceMetadataLoader,
schemaLoader: schemaLoader,
schemaLoader dclschemaloader.DCLSchemaLoader) HandlerFunc {
return func(mgr manager.Manager) admission.Handler {
return &iamValidatorHandler{
smLoader: smLoader,
serviceMetadataLoader: serviceMetadataLoader,
schemaLoader: schemaLoader,
}
}
}

Expand Down
15 changes: 9 additions & 6 deletions pkg/webhook/immutable_fields_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

Expand All @@ -63,12 +64,14 @@ var (
allowedResponse = admission.ValidationResponse(true, "admission controller passed")
)

func NewImmutableFieldsValidatorHandler(smLoader *servicemappingloader.ServiceMappingLoader, dclSchemaLoader dclschemaloader.DCLSchemaLoader, serviceMetadataLoader dclmetadata.ServiceMetadataLoader) *immutableFieldsValidatorHandler {
return &immutableFieldsValidatorHandler{
smLoader: smLoader,
tfResourceMap: provider.ResourceMap(),
dclSchemaLoader: dclSchemaLoader,
serviceMetadataLoader: serviceMetadataLoader,
func NewImmutableFieldsValidatorHandler(smLoader *servicemappingloader.ServiceMappingLoader, dclSchemaLoader dclschemaloader.DCLSchemaLoader, serviceMetadataLoader dclmetadata.ServiceMetadataLoader) HandlerFunc {
return func(mgr manager.Manager) admission.Handler {
return &immutableFieldsValidatorHandler{
smLoader: smLoader,
tfResourceMap: provider.ResourceMap(),
dclSchemaLoader: dclSchemaLoader,
serviceMetadataLoader: serviceMetadataLoader,
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/webhook/immutable_fields_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1529,7 +1529,7 @@ func TestChangesOnImmutableFieldsForDCLResource(t *testing.T) {
}
}

func assertImmutableFieldsValidatorResult(t *testing.T, v *immutableFieldsValidatorHandler, provider *schema.Provider, testCase TestCase) {
func assertImmutableFieldsValidatorResult(t *testing.T, _ HandlerFunc, provider *schema.Provider, testCase TestCase) {
r, ok := provider.ResourcesMap[testCase.TFSchemaName]
if !ok {
t.Errorf("couldn't get the schema for %v", testCase.TFSchemaName)
Expand Down Expand Up @@ -1617,7 +1617,7 @@ func TestChangesOnImmutableResourceIDField(t *testing.T) {
}
}

func newImmutableFieldsValidatorHandler(t *testing.T) *immutableFieldsValidatorHandler {
func newImmutableFieldsValidatorHandler(t *testing.T) HandlerFunc {
t.Helper()
smLoader, err := servicemappingloader.New()
if err != nil {
Expand Down
25 changes: 9 additions & 16 deletions pkg/webhook/logging_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@ import (
"context"

"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

Expand All @@ -31,12 +30,15 @@ type RequestLoggingHandler struct {
handlerName string
}

var _ inject.Client = &RequestLoggingHandler{}
type HandlerFunc func(mgr manager.Manager) admission.Handler

func NewRequestLoggingHandler(handler admission.Handler, handlerName string) *RequestLoggingHandler {
return &RequestLoggingHandler{
handler: handler,
handlerName: handlerName,
func NewRequestLoggingHandler(handlerFunc HandlerFunc, handlerName string) HandlerFunc {
return func(mgr manager.Manager) admission.Handler {
handler := handlerFunc(mgr)
return &RequestLoggingHandler{
handler: handler,
handlerName: handlerName,
}
}
}

Expand Down Expand Up @@ -65,12 +67,3 @@ func (a *RequestLoggingHandler) Handle(ctx context.Context, req admission.Reques
}
return response
}

// InjectClient is called by controller-runtime to inject a client into the handler
func (a *RequestLoggingHandler) InjectClient(c client.Client) error {
injectClient, ok := a.handler.(inject.Client)
if !ok {
return nil
}
return injectClient.InjectClient(c)
}
26 changes: 10 additions & 16 deletions pkg/webhook/management_conflict_annotation_defaulter.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (
apimachinerytypes "k8s.io/apimachinery/pkg/types"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

Expand All @@ -43,24 +43,18 @@ type managementConflictAnnotationDefaulter struct {
tfResourceMap map[string]*tfschema.Resource
}

func NewManagementConflictAnnotationDefaulter(smLoader *servicemappingloader.ServiceMappingLoader, dclSchemaLoader dclschemaloader.DCLSchemaLoader, serviceMetadataLoader dclmetadata.ServiceMetadataLoader) *managementConflictAnnotationDefaulter {
return &managementConflictAnnotationDefaulter{
smLoader: smLoader,
serviceMetadataLoader: serviceMetadataLoader,
dclSchemaLoader: dclSchemaLoader,
tfResourceMap: provider.ResourceMap(),
func NewManagementConflictAnnotationDefaulter(smLoader *servicemappingloader.ServiceMappingLoader, dclSchemaLoader dclschemaloader.DCLSchemaLoader, serviceMetadataLoader dclmetadata.ServiceMetadataLoader) HandlerFunc {
return func(mgr manager.Manager) admission.Handler {
return &managementConflictAnnotationDefaulter{
client: mgr.GetClient(),
smLoader: smLoader,
serviceMetadataLoader: serviceMetadataLoader,
dclSchemaLoader: dclSchemaLoader,
tfResourceMap: provider.ResourceMap(),
}
}
}

// managementConflictAnnotationDefaulter implements inject.Client.
var _ inject.Client = &managementConflictAnnotationDefaulter{}

// InjectClient injects the client into the managementConflictAnnotationDefaulter
func (a *managementConflictAnnotationDefaulter) InjectClient(c client.Client) error {
a.client = c
return nil
}

func (a *managementConflictAnnotationDefaulter) Handle(ctx context.Context, req admission.Request) admission.Response {
deserializer := codecs.UniversalDeserializer()
obj := &unstructured.Unstructured{}
Expand Down
20 changes: 7 additions & 13 deletions pkg/webhook/no_unknown_fields_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
apitypes "k8s.io/apimachinery/pkg/types"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

Expand All @@ -38,21 +38,15 @@ type noUnknownFieldsValidatorHandler struct {
smLoader *servicemappingloader.ServiceMappingLoader
}

// noUnknownFieldsValidatorHandler implements inject.Client.
var _ inject.Client = &noUnknownFieldsValidatorHandler{}

func NewNoUnknownFieldsValidatorHandler(smLoader *servicemappingloader.ServiceMappingLoader) *noUnknownFieldsValidatorHandler {
return &noUnknownFieldsValidatorHandler{
smLoader: smLoader,
func NewNoUnknownFieldsValidatorHandler(smLoader *servicemappingloader.ServiceMappingLoader) HandlerFunc {
return func(mgr manager.Manager) admission.Handler {
return &noUnknownFieldsValidatorHandler{
client: mgr.GetClient(),
smLoader: smLoader,
}
}
}

// InjectClient injects the client into the noUnknownFieldsValidatorHandler
func (a *noUnknownFieldsValidatorHandler) InjectClient(c client.Client) error {
a.client = c
return nil
}

func (a *noUnknownFieldsValidatorHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
deserializer := codecs.UniversalDeserializer()
obj := &unstructured.Unstructured{}
Expand Down