Tracking issue for tracking forced configuration parameters

[bart0sh]

This is a tracking issue for implementing correct handling of overriding configuration parameters. The goal of it is to list all overrides and decide how to handle them in a user-friendly and generic manner.

Currently at least some of the parameters are simply ignored, which confuses users. Example issues: #1254, #1252, #1028

Preliminary list of parameters to check::

• externalkubeletcfg.Authentication.X509.ClientCAFile
• externalkubeletcfg.Authorization.Mode
• externalkubeletcfg.Authentication.Webhook.Enabled
• externalkubeletcfg.ReadOnlyPort
• externalkubeletcfg.RotateCertificates
• externalkubeletcfg.HealthzBindAddress
• externalkubeletcfg.HealthzPort

Possible solution: Add warnings when parameters are overriden specifying a reason for override.

[fabriziopandini]

Another kubeadm override I'm aware of is here: it enforce authorization modes in the kube-apiserver overriding user ExtraArgs

[bart0sh]

Looking at default config example I can see a lot of possible candidates to be overriden in a user config in a similar way to failSwapOn(#1254). Should we consider implementing some kind of validation for user-provided config where we can check all possible overrides?

[bart0sh]

/cc @neolit123
/cc @fabriziopandini
/cc @timothysc

[neolit123]

my idea was to add some sort of an utility that accepts a slice of flag names and a "reason".

warnFieldOverrides(flags []string, reason string) {
// ... print to the user the list of flags that we are overwriting
// and why were doing that (reason).
}

we can then use this utility in key locations that do overrides.
but let's see what others think first.

[bart0sh]

@neolit123 How this will help to detect overrides made in user config (similar to failSwapOn)? For example, if user puts "makeIPTablesUtilChains = false" or "syncFrequency: 10s" or one of dozens other parameters into config and run "kubeadm init/join/upgrade/etc --config " with it. User would expect those parameters to be used and kubeadm should warn user if it's not the case.

[neolit123]

How this will help to detect overrides made in user config (similar to failSwapOn)

its manual, no detection.
there is no easy way to do detection and even if we can i don't think we should.

we basically need a wrapper that either updates + prints warning or a utility that prints that some fields were overwritten.

[bart0sh]

its manual, no detection

I thought it's a purpose of this issue to detect things like this one, no? If we can't detect this kind of changes we can only address overrides of externalkubeletcfg parameters.

[neolit123]

in my opinion we should only address the kubelet overrides, for now.
but i will leave it to others to comment too.

[fabriziopandini]

My understanding is that so far we know only two points into he code where kubeadm makes overrides one for kubelet config and one for the API servers extra args; Overall there are less then 10 attributes overwritten by kubeadm.
If that's correct, I don't see value in investing time for building a sophisticated solution for detecting what kubeadm overrides. Let's keep this simple, even if this implies less adaptability to future changes.

However I`m a little bit concerned about how we want the warning to surface to the user, because those overrides a deep in the call stack, because I don't like the Idea of having printf spreading in the code base (we already have two much of them). Any thoughts about this?

[neolit123]

However I`m a little bit concerned about how we want the warning to surface to the user, because those overrides a deep in the call stack, because I don't like the Idea of having printf spreading in the code base (we already have two much of them). Any thoughts about this?

we can use an interface that collects the problem fields in a shared bucket and print them only on the CLI side.

[bart0sh]

@fabriziopandini @neolit123 My main concern for now is silently ignored options from user config. Can you suggest how to discover which of those dozens of options there can be potentially ignored? The only method I'm aware of so far was to change options in the config, run kubeadm and see which of those have been ignored. Is there a better way?

[chuckha]

Once we gather a list of everything that is overridden (we will miss some and that's ok, we can improve with time) we will have the ability to let the user know ASAP when we're about to override a value they've set. The printfs won't be nested deep in the code and it could be on initial validation. It does require maintaining a list by hand though.

edit: @bart0sh an improved list over time is the alternative (and imo better) way to trying to figure it all out up front perfectly.

[fabriziopandini]

I'm in favour of list improved over time too.

I'm also confident that kubeadm overrides are concentrated in few points:

1. where kubeadm transforms extra args into flags for the control-plane components (already bin the radar)
2. where kubeadm defaults component config for kubelet and kue proxy (already in the radar)

Other places worth to check are the SetDynamic default functions and the function where kubelet ExtraArgs are written to file, but I don't expect the current list to grow up significantly

[bart0sh]

Yet another issue that's caused by kubelet ignoring configuration file options. Copied from slack:

Hi i try to join a node via config file:
apiVersion: kubeadm.k8s.io/v1beta1
kind: JoinConfiguration
ControlPlane: 
  localAPIEndpoint:
    advertiseAddress: 10.99.120.12
    bindPort: 6443
Discovery:
  BootstrapToken:
    APIServerEndpoint: 10.99.120.10:6443
    Token: zrnbq.o2gjhna4324sgpis
    CACertHashes: 
      - 38966069b81fc8f3234553b3d6be7cf3dd233fe1adbb3803b63db2d2f17aa836```
but still getting
   root@k8s-master-2:~# kubeadm join --config kubeadm-config-k8s-master-2.yaml
discovery: Invalid value: "": bootstrapToken or file must be set

[neolit123]

the yaml unmarsal is case sensitive.
some of the fields in the above example are uppercase where they shouldn't be.

https://godoc.org/k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1

[bart0sh]

/help-wanted

[astrieanna]

Looking to contribute to this issue, but have a couple clarifying questions. First, I want to make sure I understand the goal.

Current Behavior:
When I set an option in kubelet config that has a required default in kubeadm, kubeadm will silently override the value.

Desired Behavior:
When I set an option in kubelet config that has a required default in kubeadm, kubeadm will _____.
a) warn and continue
b) error and stop

The discussion here seems to have been option (a). Has erroring and refusing to continue if the user configures these ignored options been considered?

Because kubeadm is a tool used by other tools to automate cluster creation, and warnings are rarely surfaced in automated environments, it can be easy for a user to believe that their configuration has been applied despite kubeadm printing a warning. Errors are surfaced and provide immediate feedback.

While the kubelet config passed in is for kubelet, in this case is also has be valid for kubeadm, and setting these values (other than to the required defaults) makes it an invalid kubeadm kubelet config.

[neolit123]

in today's kubeadm office hours we decided that kubeadm should not stomp the settings and we should allow the user configuration.
possibly add warnings if they are different from the defaults.

[neolit123]

@bart0sh feel free to work on this if you have the bandwidth...
we finally settled that it's OK to allow user values...

[chrischdi]

Hi,
we also have interest in implementing this. Our use-case is regarding the externalkubeletcfg.Authorization.Mode parameter.

We configure authorization-mode: "Node,Webhook,RBAC" as apiserver extra arg.
Kubeadm enforces Node,RBAC,Webhook instead in the pod-manifest

Use case:
We are using OPA and the policy-controller (now gatekeeper) as webhook here to block additional requests which we are not able to block via validatingwebhook.

Additional information: https://kubernetes.slack.com/archives/C13J86Z63/p1562355381313100

[neolit123]

@chrischdi hi, if you are willing to work on this please go ahead.

a single PR can be scoped in the following way:

• create an utility function that accepts a parameter name/location (e.g. authorization-mode) and prints a warning (using klog.Warningf) in the lines of:

Warning: the recommended value for PARAM-NAME for COMPONENT-NAME is VALUE

• COMPONENT-NAME can be kube-apiserver, kubelet, etc.
• possibly place this function under /utils.
• don't override user values for the kubelet, show warnings if the user has passed values different from the default:
  https://github.com/kubernetes/kubernetes/blob/23111ad414933027d0e2e297461477d0f9d267fd/cmd/kubeadm/app/componentconfigs/defaults.go#L88-L109
• don't override user values for the api-server:
  https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/phases/controlplane/manifests.go

[jfbai]

/assign

@neolit123 I will work on this issue, thanks a lot.

[neolit123]

fixed by: kubernetes/kubernetes#81903
thanks

[chrischdi]

@neolit123 one question: kubernetes/kubernetes#81903 fixes this issue for kubelet and kube-proxy configurations.

There are still enforced values for example for the pod manifests:
See kube-apiserver commands and especially here

So it is about the following config part:

apiVersion: kubeadm.k8s.io/v1beta1
kind: ClusterConfiguration
apiServer:
  extraArgs:
    authorization-mode: Node,Webhook,RBAC
...

Should this have been in-scope for this issue?

[neolit123]

@chrischdi kubeadm will still deploy using it's harcoded manifests for the static pods, but you can apply kustomizations to them (alpha in 1.16)
see:
#1379