Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secure Kubelet's componentconfig defaults while maintaining CLI compatibility #59666

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ func TestValidateKubeletConfiguration(t *testing.T) {
OOMScoreAdj: utilpointer.Int32Ptr(-999),
PodsPerCore: 100,
Port: 65535,
ReadOnlyPort: utilpointer.Int32Ptr(0),
ReadOnlyPort: 0,
RegistryBurst: 10,
RegistryPullQPS: utilpointer.Int32Ptr(5),
HairpinMode: "promiscuous-bridge",
Expand Down Expand Up @@ -617,7 +617,7 @@ func TestValidateKubeletConfiguration(t *testing.T) {
OOMScoreAdj: utilpointer.Int32Ptr(-1001),
PodsPerCore: -10,
Port: 0,
ReadOnlyPort: utilpointer.Int32Ptr(-10),
ReadOnlyPort: -10,
RegistryBurst: -10,
RegistryPullQPS: utilpointer.Int32Ptr(-10),
},
Expand Down
1 change: 1 addition & 0 deletions cmd/kubelet/app/options/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ go_library(
"//pkg/kubelet/apis/kubeletconfig/validation:go_default_library",
"//pkg/kubelet/config:go_default_library",
"//pkg/kubelet/types:go_default_library",
"//pkg/master/ports:go_default_library",
"//pkg/util/taints:go_default_library",
"//pkg/version/verflag:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
Expand Down
22 changes: 21 additions & 1 deletion cmd/kubelet/app/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
kubeletconfigvalidation "k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig/validation"
"k8s.io/kubernetes/pkg/kubelet/config"
kubetypes "k8s.io/kubernetes/pkg/kubelet/types"
"k8s.io/kubernetes/pkg/master/ports"
utiltaints "k8s.io/kubernetes/pkg/util/taints"
)

Expand All @@ -65,6 +66,9 @@ type KubeletFlags struct {
// run those in addition to the pods specified by the local manifest, and exit.
RunOnce bool

// enableServer enables the Kubelet's server
EnableServer bool

// HostnameOverride is the hostname used to identify the kubelet instead
// of the actual hostname.
HostnameOverride string
Expand Down Expand Up @@ -214,6 +218,7 @@ func NewKubeletFlags() *KubeletFlags {
}

return &KubeletFlags{
EnableServer: true,
ContainerRuntimeOptions: *NewContainerRuntimeOptions(),
CertDirectory: "/var/lib/kubelet/pki",
RootDirectory: v1alpha1.DefaultRootDir,
Expand Down Expand Up @@ -264,9 +269,24 @@ func NewKubeletConfiguration() (*kubeletconfig.KubeletConfiguration, error) {
if err := scheme.Convert(versioned, config, nil); err != nil {
return nil, err
}
applyLegacyDefaults(config)
return config, nil
}

// applyLegacyDefaults applies legacy default values to the KubeletConfiguration in order to
// preserve the command line API. This is used to construct the baseline default KubeletConfiguration
// before the first round of flag parsing.
func applyLegacyDefaults(kc *kubeletconfig.KubeletConfiguration) {
// --anonymous-auth
kc.Authentication.Anonymous.Enabled = true
// --authentication-token-webhook
kc.Authentication.Webhook.Enabled = false
// --authorization-mode
kc.Authorization.Mode = kubeletconfig.KubeletAuthorizationModeAlwaysAllow
// --read-only-port
kc.ReadOnlyPort = ports.KubeletReadOnlyPort
}

// KubeletServer encapsulates all of the parameters necessary for starting up
// a kubelet. These can either be set via command line or directly.
type KubeletServer struct {
Expand Down Expand Up @@ -322,6 +342,7 @@ func (f *KubeletFlags) AddFlags(fs *pflag.FlagSet) {
fs.Float64Var(&f.ChaosChance, "chaos-chance", f.ChaosChance, "If > 0.0, introduce random client errors and latency. Intended for testing.")

fs.BoolVar(&f.RunOnce, "runonce", f.RunOnce, "If true, exit after spawning pods from local manifests or remote urls. Exclusive with --enable-server")
fs.BoolVar(&f.EnableServer, "enable-server", f.EnableServer, "Enable the Kubelet's server")

fs.StringVar(&f.HostnameOverride, "hostname-override", f.HostnameOverride, "If non-empty, will use this string as identification instead of the actual hostname.")

Expand Down Expand Up @@ -407,7 +428,6 @@ func AddKubeletConfigFlags(fs *pflag.FlagSet, c *kubeletconfig.KubeletConfigurat
fs.DurationVar(&c.HTTPCheckFrequency.Duration, "http-check-frequency", c.HTTPCheckFrequency.Duration, "Duration between checking http for new data")
fs.StringVar(&c.ManifestURL, "manifest-url", c.ManifestURL, "URL for accessing the container manifest")
fs.Var(flag.NewColonSeparatedMultimapStringString(&c.ManifestURLHeader), "manifest-url-header", "Comma-separated list of HTTP headers to use when accessing the manifest URL. Multiple headers with the same name will be added in the same order provided. This flag can be repeatedly invoked. For example: `--manifest-url-header 'a:hello,b:again,c:world' --manifest-url-header 'b:beautiful'`")
fs.BoolVar(&c.EnableServer, "enable-server", c.EnableServer, "Enable the Kubelet's server")
fs.Var(componentconfig.IPVar{Val: &c.Address}, "address", "The IP address for the Kubelet to serve on (set to `0.0.0.0` for all IPv4 interfaces and `::` for all IPv6 interfaces)")
fs.Int32Var(&c.Port, "port", c.Port, "The port for the Kubelet to serve on.")
fs.Int32Var(&c.ReadOnlyPort, "read-only-port", c.ReadOnlyPort, "The read-only port for the Kubelet to serve on with no authentication/authorization (set to 0 to disable)")
Expand Down
6 changes: 3 additions & 3 deletions cmd/kubelet/app/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -930,18 +930,18 @@ func RunKubelet(kubeFlags *options.KubeletFlags, kubeCfg *kubeletconfiginternal.
}
glog.Infof("Started kubelet as runonce")
} else {
startKubelet(k, podCfg, kubeCfg, kubeDeps)
startKubelet(k, podCfg, kubeCfg, kubeDeps, kubeFlags.EnableServer)
glog.Infof("Started kubelet")
}
return nil
}

func startKubelet(k kubelet.Bootstrap, podCfg *config.PodConfig, kubeCfg *kubeletconfiginternal.KubeletConfiguration, kubeDeps *kubelet.Dependencies) {
func startKubelet(k kubelet.Bootstrap, podCfg *config.PodConfig, kubeCfg *kubeletconfiginternal.KubeletConfiguration, kubeDeps *kubelet.Dependencies, enableServer bool) {
// start the kubelet
go wait.Until(func() { k.Run(podCfg.Updates()) }, 0, wait.NeverStop)

// start the kubelet server
if kubeCfg.EnableServer {
if enableServer {
go wait.Until(func() {
k.ListenAndServe(net.ParseIP(kubeCfg.Address), uint(kubeCfg.Port), kubeDeps.TLSOptions, kubeDeps.Auth, kubeCfg.EnableDebuggingHandlers, kubeCfg.EnableContentionProfiling)
}, 0, wait.NeverStop)
Expand Down
1 change: 0 additions & 1 deletion pkg/kubelet/apis/kubeletconfig/fuzzer/fuzzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ func Funcs(codecs runtimeserializer.CodecFactory) []interface{} {
obj.EventRecordQPS = 5
obj.EnableControllerAttachDetach = true
obj.EnableDebuggingHandlers = true
obj.EnableServer = true
obj.FileCheckFrequency = metav1.Duration{Duration: 20 * time.Second}
obj.HealthzBindAddress = "127.0.0.1"
obj.HealthzPort = 10248
Expand Down
1 change: 0 additions & 1 deletion pkg/kubelet/apis/kubeletconfig/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ var (
"EnableContentionProfiling",
"EnableControllerAttachDetach",
"EnableDebuggingHandlers",
"EnableServer",
"EnforceNodeAllocatable[*]",
"EventBurst",
"EventRecordQPS",
Expand Down
2 changes: 0 additions & 2 deletions pkg/kubelet/apis/kubeletconfig/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ type KubeletConfiguration struct {
// manifestURLHeader is the HTTP header to use when accessing the manifest
// URL, with the key separated from the value with a ':', as in 'key:value'
ManifestURLHeader map[string][]string
// enableServer enables the Kubelet's server
EnableServer bool
// address is the IP address for the Kubelet to serve on (set to 0.0.0.0
// for all interfaces)
Address string
Expand Down
12 changes: 3 additions & 9 deletions pkg/kubelet/apis/kubeletconfig/v1alpha1/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,16 @@ func SetDefaults_KubeletConfiguration(obj *KubeletConfiguration) {
obj.ConfigTrialDuration = &metav1.Duration{Duration: 10 * time.Minute}
}
if obj.Authentication.Anonymous.Enabled == nil {
obj.Authentication.Anonymous.Enabled = boolVar(true)
obj.Authentication.Anonymous.Enabled = boolVar(false)
}
if obj.Authentication.Webhook.Enabled == nil {
obj.Authentication.Webhook.Enabled = boolVar(false)
obj.Authentication.Webhook.Enabled = boolVar(true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only work if you have a kubeconfig specified, right? Do we need to make this conditional on whether you are in standalone mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't be a silent failure: https://github.com/kubernetes/kubernetes/blob/master/cmd/kubelet/app/auth.go#L73

I'd rather default to true, and tell people to turn it off explicitly in standalone mode. Config defaults changing depending on whether you got a --kubeconfig feels a little too implicit to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

}
if obj.Authentication.Webhook.CacheTTL == zeroDuration {
obj.Authentication.Webhook.CacheTTL = metav1.Duration{Duration: 2 * time.Minute}
}
if obj.Authorization.Mode == "" {
obj.Authorization.Mode = KubeletAuthorizationModeAlwaysAllow
obj.Authorization.Mode = KubeletAuthorizationModeWebhook
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with this

}
if obj.Authorization.Webhook.CacheAuthorizedTTL == zeroDuration {
obj.Authorization.Webhook.CacheAuthorizedTTL = metav1.Duration{Duration: 5 * time.Minute}
Expand Down Expand Up @@ -92,9 +92,6 @@ func SetDefaults_KubeletConfiguration(obj *KubeletConfiguration) {
if obj.EnableDebuggingHandlers == nil {
obj.EnableDebuggingHandlers = boolVar(true)
}
if obj.EnableServer == nil {
obj.EnableServer = boolVar(true)
}
if obj.FileCheckFrequency == zeroDuration {
obj.FileCheckFrequency = metav1.Duration{Duration: 20 * time.Second}
}
Expand Down Expand Up @@ -145,9 +142,6 @@ func SetDefaults_KubeletConfiguration(obj *KubeletConfiguration) {
if obj.Port == 0 {
obj.Port = ports.KubeletPort
}
if obj.ReadOnlyPort == nil {
obj.ReadOnlyPort = utilpointer.Int32Ptr(ports.KubeletReadOnlyPort)
}
if obj.RegistryBurst == 0 {
obj.RegistryBurst = 10
}
Expand Down
6 changes: 2 additions & 4 deletions pkg/kubelet/apis/kubeletconfig/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,14 @@ type KubeletConfiguration struct {
// manifestURLHeader is the HTTP header to use when accessing the manifest
// URL, with the key separated from the value with a ':', as in 'key:value'
ManifestURLHeader map[string][]string `json:"manifestURLHeader"`
// enableServer enables the Kubelet's server
EnableServer *bool `json:"enableServer"`
// address is the IP address for the Kubelet to serve on (set to 0.0.0.0
// for all interfaces)
Address string `json:"address"`
// port is the port for the Kubelet to serve on.
Port int32 `json:"port"`
// readOnlyPort is the read-only port for the Kubelet to serve on with
// no authentication/authorization (set to 0 to disable)
ReadOnlyPort *int32 `json:"readOnlyPort"`
// no authentication/authorization. Disabled (set to 0) by default.
ReadOnlyPort int32 `json:"readOnlyPort"`
// tlsCertFile is the file containing x509 Certificate for HTTPS. (CA cert,
// if any, concatenated after server cert). If tlsCertFile and
// tlsPrivateKeyFile are not provided, a self-signed certificate
Expand Down
14 changes: 2 additions & 12 deletions pkg/kubelet/apis/kubeletconfig/v1alpha1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 0 additions & 18 deletions pkg/kubelet/apis/kubeletconfig/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/kubemark/hollow_kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ func GetHollowKubeletConfig(

// Flags struct
f := options.NewKubeletFlags()
f.EnableServer = true
f.RootDirectory = testRootDir
f.HostnameOverride = nodeName
f.MinimumGCAge = metav1.Duration{Duration: 1 * time.Minute}
Expand Down Expand Up @@ -144,7 +145,6 @@ func GetHollowKubeletConfig(
c.CPUCFSQuota = true
c.EnableControllerAttachDetach = false
c.EnableDebuggingHandlers = true
c.EnableServer = true
c.CgroupsPerQOS = false
// hairpin-veth is used to allow hairpin packets. Note that this deviates from
// what the "real" kubelet currently does, because there's no way to
Expand Down