kubernetes-sigs / kubernetes-sigs/controller-runtime

Builder's `For`, `Owns` and `Watches` methods overwrite multiple options

Open
#945 5 comments 0 reactions 0 assignees View on GitHub
lifecycle/frozen
Dominant language
Go
Stars
3k
Forks
1.3k
Avg merge
2d 5h
Merged PRs (30d)
14

Description

The `Builder` struct's `For`, `Owns` and `Watches` methods all follow the same pattern of having a variable number of modifying `Option` structs as the final parameter to the method. Here is the code for the `For` method:

```go
// For defines the type of Object being *reconciled*, and configures the ControllerManagedBy to respond to create / delete /
// update events by *reconciling the object*.
// This is the equivalent of calling
// Watches(&source.Kind{Type: apiType}, &handler.EnqueueRequestForObject{})
func (blder *Builder) For(object runtime.Object, opts ...ForOption) *Builder {
input := ForInput{object: object}
for _, opt := range opts {
opt.ApplyToFor(&input)
}

blder.forInput = input
return blder
}
```

The problem with the above is if you call `For` with more than one `ForOption`, the builder's `forInput` member ends up containing only the predicates contained in the last `ForOption`. The reason is because the `ForOption.ApplyToFor` *assigns* the `ForOption.predicates` member to the supplied `ForInput.predicates` member (note: `ForOption` is an interface that is implemented by the `Predicates` struct):

```go
// ApplyToFor applies this configuration to the given ForInput options.
func (w Predicates) ApplyToFor(opts *ForInput) {
opts.predicates = w.predicates
}
```

The solution is to append `w.predicates` to the supplied `opt.predicates` instead of assigning/overwriting the value of `opts.predicates`:

```go
// ApplyToFor applies this configuration to the given ForInput options.
func (w Predicates) ApplyToFor(opts *ForInput) {
opts.predicates = append(opts.predicates, w.predicates...)
}
```

Contributor guide

Open the contributing guide

Research direction

Locate the Builder's For, Owns, and Watches entry points and the corresponding ForOption, OwnsOption, and WatchesOption implementations. Compare how each ApplyTo method handles multiple options, then verify that predicates from every supplied option are retained rather than overwritten using the repository's relevant tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.