kubermatic / kubermatic/machine-controller

defaultAndValidateMachineSpec() only validates, doesn't default

Open
#1,670 8 comments 0 reactions 0 assignees View on GitHub
kind/bug lifecycle/frozen sig/cluster-management
Dominant language
Go
Stars
326
Forks
138
Avg merge
2d 46m
Merged PRs (30d)
12

Description

Unless I'm very confused right now, this code in defaultAndValidateMachineSpec() (part the of the mutating webhooks)

https://github.com/kubermatic/machine-controller/blob/94e2005cbd847a05e72d04ce97dc2d3bab59fa1c/pkg/admission/machines.go#L185

...only updates the passed-in pointer to the to-be-defaulted MachineSpec, but doesn't update the MachineSpec it points to. So the updated object will be garbage collected and the caller's MachineSpec will not be updated (i.e. defaulted).
Changing the line to something like `*spec = defaultedSpec` should fix it (or changing some function signatures so we pass pointers rather than values). I'm not sure about the implications because it seems like this bug has been in there since 2018. So maybe nobody really needed the provider-specific defaulting functionality?

Contributor guide

Open the contributing guide

Research direction

Start in pkg/admission/machines.go around defaultAndValidateMachineSpec() at the linked line, then trace how its MachineSpec pointer is used by the mutating webhook caller. Confirm whether the caller's MachineSpec remains unchanged after defaulting; done means provider-specific defaults are present on the returned or mutated object and the existing behavior is covered by an appropriate regression test.

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
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.