kubernetes-sigs / kubernetes-sigs/controller-runtime
Builder's `For`, `Owns` and `Watches` methods overwrite multiple options
- 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
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