Skip to content

Commit

Permalink
Merge pull request #958 from justinsb/stop_using_inject
Browse files Browse the repository at this point in the history
refactor: stop using inject framework in controller-runtime
  • Loading branch information
google-oss-prow[bot] committed Oct 25, 2023
2 parents 2467649 + 9802eaf commit 965bc17
Show file tree
Hide file tree
Showing 15 changed files with 104 additions and 109 deletions.
3 changes: 2 additions & 1 deletion config/tests/samples/create/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,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

0 comments on commit 965bc17

Please sign in to comment.