Skip to content

Commit

Permalink
⚠️ Remove pkg/inject
Browse files Browse the repository at this point in the history
Signed-off-by: Vince Prignano <[email protected]>
  • Loading branch information
vincepri committed Jan 20, 2023
1 parent d4a1690 commit 9c26a1d
Show file tree
Hide file tree
Showing 47 changed files with 127 additions and 1,030 deletions.
2 changes: 0 additions & 2 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ issues:
- Subprocess launch(ed with variable|ing should be audited)
- (G204|G104|G307)
- "ST1000: at least one file in a package should have a package comment"
- "SA1019: \"sigs.k8s.io/controller-runtime/pkg/runtime/inject\""
- "SA1019: inject.*"
exclude-rules:
- linters:
- gosec
Expand Down
2 changes: 1 addition & 1 deletion alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ var (
// The logger, when used with controllers, can be expected to contain basic information about the object
// that's being reconciled like:
// - `reconciler group` and `reconciler kind` coming from the For(...) object passed in when building a controller.
// - `name` and `namespace` injected from the reconciliation request.
// - `name` and `namespace` from the reconciliation request.
//
// This is meant to be used with the context supplied in a struct that satisfies the Reconciler interface.
LoggerFrom = log.FromContext
Expand Down
2 changes: 1 addition & 1 deletion doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ limitations under the License.
//
// Every controller and webhook is ultimately run by a Manager (pkg/manager). A
// manager is responsible for running controllers and webhooks, and setting up
// common dependencies (pkg/runtime/inject), like shared caches and clients, as
// common dependencies, like shared caches and clients, as
// well as managing leader election (pkg/leaderelection). Managers are
// generally configured to gracefully shut down controllers on pod termination
// by wiring up a signal handler (pkg/manager/signals).
Expand Down
2 changes: 0 additions & 2 deletions example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
// ReplicaSetReconciler.
//
// * Start the application.
// TODO(pwittrock): Update this example when we have better dependency injection support.
func Example() {
var log = ctrl.Log.WithName("builder-examples")

Expand Down Expand Up @@ -75,7 +74,6 @@ func Example() {
// ReplicaSetReconciler.
//
// * Start the application.
// TODO(pwittrock): Update this example when we have better dependency injection support.
func Example_updateLeaderElectionDurations() {
var log = ctrl.Log.WithName("builder-examples")
leaseDuration := 100 * time.Second
Expand Down
5 changes: 3 additions & 2 deletions examples/builtins/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/manager/signals"
"sigs.k8s.io/controller-runtime/pkg/source"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

func init() {
Expand Down Expand Up @@ -76,8 +77,8 @@ func main() {
hookServer := mgr.GetWebhookServer()

entryLog.Info("registering webhooks to the webhook server")
hookServer.Register("/mutate-v1-pod", &webhook.Admission{Handler: &podAnnotator{Client: mgr.GetClient()}})
hookServer.Register("/validate-v1-pod", &webhook.Admission{Handler: &podValidator{Client: mgr.GetClient()}})
hookServer.Register("/mutate-v1-pod", &webhook.Admission{Handler: &podAnnotator{Client: mgr.GetClient(), decoder: admission.NewDecoder(mgr.GetScheme())}})
hookServer.Register("/validate-v1-pod", &webhook.Admission{Handler: &podValidator{Client: mgr.GetClient(), decoder: admission.NewDecoder(mgr.GetScheme())}})

entryLog.Info("starting manager")
if err := mgr.Start(signals.SetupSignalHandler()); err != nil {
Expand Down
9 changes: 0 additions & 9 deletions examples/builtins/mutatingwebhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,3 @@ func (a *podAnnotator) Handle(ctx context.Context, req admission.Request) admiss

return admission.PatchResponseFromRaw(req.Object.Raw, marshaledPod)
}

// podAnnotator implements admission.DecoderInjector.
// A decoder will be automatically injected.

// InjectDecoder injects the decoder.
func (a *podAnnotator) InjectDecoder(d *admission.Decoder) error {
a.decoder = d
return nil
}
9 changes: 0 additions & 9 deletions examples/builtins/validatingwebhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,3 @@ func (v *podValidator) Handle(ctx context.Context, req admission.Request) admiss

return admission.Allowed("")
}

// podValidator implements admission.DecoderInjector.
// A decoder will be automatically injected.

// InjectDecoder injects the decoder.
func (v *podValidator) InjectDecoder(d *admission.Decoder) error {
v.decoder = d
return nil
}
9 changes: 3 additions & 6 deletions pkg/builder/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ func ExampleBuilder() {
ControllerManagedBy(mgr). // Create the ControllerManagedBy
For(&appsv1.ReplicaSet{}). // ReplicaSet is the Application API
Owns(&corev1.Pod{}). // ReplicaSet owns Pods created by it
Complete(&ReplicaSetReconciler{})
Complete(&ReplicaSetReconciler{
Client: mgr.GetClient(),
})
if err != nil {
log.Error(err, "could not create controller")
os.Exit(1)
Expand Down Expand Up @@ -155,8 +157,3 @@ func (a *ReplicaSetReconciler) Reconcile(ctx context.Context, req reconcile.Requ

return reconcile.Result{}, nil
}

func (a *ReplicaSetReconciler) InjectClient(c client.Client) error {
a.Client = c
return nil
}
8 changes: 4 additions & 4 deletions pkg/builder/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,10 @@ func (blder *WebhookBuilder) registerDefaultingWebhook() {

func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
if defaulter := blder.withDefaulter; defaulter != nil {
return admission.WithCustomDefaulter(blder.apiType, defaulter).WithRecoverPanic(blder.recoverPanic)
return admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, defaulter).WithRecoverPanic(blder.recoverPanic)
}
if defaulter, ok := blder.apiType.(admission.Defaulter); ok {
return admission.DefaultingWebhookFor(defaulter).WithRecoverPanic(blder.recoverPanic)
return admission.DefaultingWebhookFor(blder.mgr.GetScheme(), defaulter).WithRecoverPanic(blder.recoverPanic)
}
log.Info(
"skip registering a mutating webhook, object does not implement admission.Defaulter or WithDefaulter wasn't called",
Expand All @@ -194,10 +194,10 @@ func (blder *WebhookBuilder) registerValidatingWebhook() {

func (blder *WebhookBuilder) getValidatingWebhook() *admission.Webhook {
if validator := blder.withValidator; validator != nil {
return admission.WithCustomValidator(blder.apiType, validator).WithRecoverPanic(blder.recoverPanic)
return admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, validator).WithRecoverPanic(blder.recoverPanic)
}
if validator, ok := blder.apiType.(admission.Validator); ok {
return admission.ValidatingWebhookFor(validator).WithRecoverPanic(blder.recoverPanic)
return admission.ValidatingWebhookFor(blder.mgr.GetScheme(), validator).WithRecoverPanic(blder.recoverPanic)
}
log.Info(
"skip registering a validating webhook, object does not implement admission.Validator or WithValidator wasn't called",
Expand Down
34 changes: 9 additions & 25 deletions pkg/builder/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,6 @@ func runTests(admissionReviewVersion string) {

ctx, cancel := context.WithCancel(context.Background())
cancel()
// TODO: we may want to improve it to make it be able to inject dependencies,
// but not always try to load certs and return not found error.
err = svr.Start(ctx)
if err != nil && !os.IsNotExist(err) {
ExpectWithOffset(1, err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -191,8 +189,6 @@ func runTests(admissionReviewVersion string) {

ctx, cancel := context.WithCancel(context.Background())
cancel()
// TODO: we may want to improve it to make it be able to inject dependencies,
// but not always try to load certs and return not found error.
err = svr.Start(ctx)
if err != nil && !os.IsNotExist(err) {
ExpectWithOffset(1, err).NotTo(HaveOccurred())
Expand All @@ -208,7 +204,7 @@ func runTests(admissionReviewVersion string) {
By("sanity checking the response contains reasonable fields")
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`))
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":500`))
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"message":"panic: injected panic [recovered]`))
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"message":"panic: fake panic test [recovered]`))
})

It("should scaffold a defaulting webhook with a custom defaulter", func() {
Expand Down Expand Up @@ -260,8 +256,6 @@ func runTests(admissionReviewVersion string) {

ctx, cancel := context.WithCancel(context.Background())
cancel()
// TODO: we may want to improve it to make it be able to inject dependencies,
// but not always try to load certs and return not found error.
err = svr.Start(ctx)
if err != nil && !os.IsNotExist(err) {
ExpectWithOffset(1, err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -337,8 +331,6 @@ func runTests(admissionReviewVersion string) {

ctx, cancel := context.WithCancel(context.Background())
cancel()
// TODO: we may want to improve it to make it be able to inject dependencies,
// but not always try to load certs and return not found error.
err = svr.Start(ctx)
if err != nil && !os.IsNotExist(err) {
ExpectWithOffset(1, err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -411,8 +403,6 @@ func runTests(admissionReviewVersion string) {

ctx, cancel := context.WithCancel(context.Background())
cancel()
// TODO: we may want to improve it to make it be able to inject dependencies,
// but not always try to load certs and return not found error.
err = svr.Start(ctx)
if err != nil && !os.IsNotExist(err) {
ExpectWithOffset(1, err).NotTo(HaveOccurred())
Expand All @@ -430,7 +420,7 @@ func runTests(admissionReviewVersion string) {
By("sanity checking the response contains reasonable field")
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`))
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":500`))
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"message":"panic: injected panic [recovered]`))
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"message":"panic: fake panic test [recovered]`))
})

It("should scaffold a validating webhook with a custom validator", func() {
Expand Down Expand Up @@ -463,12 +453,12 @@ func runTests(admissionReviewVersion string) {
"kind":{
"group":"foo.test.org",
"version":"v1",
"kind":"TestDefaulter"
"kind":"TestValidator"
},
"resource":{
"group":"foo.test.org",
"version":"v1",
"resource":"testdefaulter"
"resource":"testvalidator"
},
"namespace":"default",
"name":"foo",
Expand All @@ -484,8 +474,6 @@ func runTests(admissionReviewVersion string) {

ctx, cancel := context.WithCancel(context.Background())
cancel()
// TODO: we may want to improve it to make it be able to inject dependencies,
// but not always try to load certs and return not found error.
err = svr.Start(ctx)
if err != nil && !os.IsNotExist(err) {
ExpectWithOffset(1, err).NotTo(HaveOccurred())
Expand All @@ -511,7 +499,7 @@ func runTests(admissionReviewVersion string) {
By("sanity checking the response contains reasonable field")
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`))
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`))
EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaulter"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`))
EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`))
})

It("should scaffold defaulting and validating webhooks if the type implements both Defaulter and Validator interfaces", func() {
Expand Down Expand Up @@ -558,8 +546,6 @@ func runTests(admissionReviewVersion string) {

ctx, cancel := context.WithCancel(context.Background())
cancel()
// TODO: we may want to improve it to make it be able to inject dependencies,
// but not always try to load certs and return not found error.
err = svr.Start(ctx)
if err != nil && !os.IsNotExist(err) {
ExpectWithOffset(1, err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -636,8 +622,6 @@ func runTests(admissionReviewVersion string) {
}`)

cancel()
// TODO: we may want to improve it to make it be able to inject dependencies,
// but not always try to load certs and return not found error.
err = svr.Start(ctx)
if err != nil && !os.IsNotExist(err) {
ExpectWithOffset(1, err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -724,7 +708,7 @@ func (*TestDefaulterList) DeepCopyObject() runtime.Object { return nil }

func (d *TestDefaulter) Default() {
if d.Panic {
panic("injected panic")
panic("fake panic test")
}
if d.Replica < 2 {
d.Replica = 2
Expand Down Expand Up @@ -767,7 +751,7 @@ var _ admission.Validator = &TestValidator{}

func (v *TestValidator) ValidateCreate() error {
if v.Panic {
panic("injected panic")
panic("fake panic test")
}
if v.Replica < 0 {
return errors.New("number of replica should be greater than or equal to 0")
Expand All @@ -777,7 +761,7 @@ func (v *TestValidator) ValidateCreate() error {

func (v *TestValidator) ValidateUpdate(old runtime.Object) error {
if v.Panic {
panic("injected panic")
panic("fake panic test")
}
if v.Replica < 0 {
return errors.New("number of replica should be greater than or equal to 0")
Expand All @@ -792,7 +776,7 @@ func (v *TestValidator) ValidateUpdate(old runtime.Object) error {

func (v *TestValidator) ValidateDelete() error {
if v.Panic {
panic("injected panic")
panic("fake panic test")
}
if v.Replica > 0 {
return errors.New("number of replica should be less than or equal to 0 to delete")
Expand Down
8 changes: 1 addition & 7 deletions pkg/cluster/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,8 @@ type cluster struct {
// config is the rest.config used to talk to the apiserver. Required.
config *rest.Config

// scheme is the scheme injected into Controllers, EventHandlers, Sources and Predicates. Defaults
// to scheme.scheme.
scheme *runtime.Scheme

cache cache.Cache

// TODO(directxman12): Provide an escape hatch to get individual indexers
// client is the client injected into Controllers (and EventHandlers, Sources and Predicates).
cache cache.Cache
client client.Client

// apiReader is the reader that will make requests to the api server and not the cache.
Expand Down
6 changes: 0 additions & 6 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,6 @@ func (d *DeferredFileLoader) OfKind(obj ControllerManagerConfiguration) *Deferre
return d
}

// InjectScheme will configure the scheme to be used for decoding the file.
func (d *DeferredFileLoader) InjectScheme(scheme *runtime.Scheme) error {
d.scheme = scheme
return nil
}

// loadFile is used from the mutex.Once to load the file.
func (d *DeferredFileLoader) loadFile() {
if d.scheme == nil {
Expand Down
33 changes: 1 addition & 32 deletions pkg/config/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,41 +44,10 @@ func ExampleFile() {
}

// This example will load the file from a custom path.
func ExampleDeferredFileLoader_atPath() {
func ExampleFile_atPath() {
loader := config.File().AtPath("/var/run/controller-runtime/config.yaml")
if _, err := loader.Complete(); err != nil {
fmt.Println("failed to load config")
os.Exit(1)
}
}

// This example sets up loader with a custom scheme.
func ExampleDeferredFileLoader_injectScheme() {
loader := config.File()
err := loader.InjectScheme(scheme)
if err != nil {
fmt.Println("failed to inject scheme")
os.Exit(1)
}

_, err = loader.Complete()
if err != nil {
fmt.Println("failed to load config")
os.Exit(1)
}
}

// This example sets up the loader with a custom scheme and custom type.
func ExampleDeferredFileLoader_ofKind() {
loader := config.File().OfKind(&v1alpha1.CustomControllerManagerConfiguration{})
err := loader.InjectScheme(scheme)
if err != nil {
fmt.Println("failed to inject scheme")
os.Exit(1)
}
_, err = loader.Complete()
if err != nil {
fmt.Println("failed to load config")
os.Exit(1)
}
}
2 changes: 1 addition & 1 deletion pkg/internal/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ var _ = Describe("controller", func() {
It("should process events from source.Channel", func() {
// channel to be closed when event is processed
processed := make(chan struct{})
// source channel to be injected
// source channel
ch := make(chan event.GenericEvent, 1)

ctx, cancel := context.WithCancel(context.TODO())
Expand Down
2 changes: 1 addition & 1 deletion pkg/internal/testing/process/process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ var _ = Describe("Start method", func() {
echo 'i started' >&2
`,
}
processState.StartTimeout = 1 * time.Second
processState.StartTimeout = 5 * time.Second

Expect(processState.Start(stdout, stderr)).To(Succeed())
Eventually(processState.Exited).Should(BeTrue())
Expand Down
Loading

0 comments on commit 9c26a1d

Please sign in to comment.