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.

Sign up for GitHub

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

Jump to bottom

Always enable RBAC in kubeadm and make a pkg with authorization constants #41810

Merged
merged 2 commits into from
Feb 23, 2017
Merged

Always enable RBAC in kubeadm and make a pkg with authorization constants #41810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ab344da
Move the authorization mode constants into a separate package
luxas Feb 23, 2017
3c322d0
kubeadm: Always enable RBAC, validate authz mode and improve the code…
luxas Feb 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/kubeadm/app/apis/kubeadm/validation/BUILD
View file
Open in desktop
Expand Up @@ -15,6 +15,7 @@ go_library(
deps = [
"//cmd/kubeadm/app/apis/kubeadm:go_default_library",
"//cmd/kubeadm/app/constants:go_default_library",
"//pkg/kubeapiserver/authorizer/modes:go_default_library",
"//pkg/registry/core/service/ipallocator:go_default_library",
"//vendor:k8s.io/apimachinery/pkg/util/validation/field",
],
Expand Down
11 changes: 10 additions & 1 deletion cmd/kubeadm/app/apis/kubeadm/validation/validation.go
View file
Open in desktop
Expand Up @@ -22,6 +22,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
authzmodes "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
"k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
)

Expand All @@ -43,8 +44,9 @@ var cloudproviders = []string{
func ValidateMasterConfiguration(c *kubeadm.MasterConfiguration) field.ErrorList {
allErrs := field.ErrorList{}
allErrs = append(allErrs, ValidateDiscovery(&c.Discovery, field.NewPath("discovery"))...)
allErrs = append(allErrs, ValidateDiscovery(&c.Discovery, field.NewPath("service subnet"))...)
allErrs = append(allErrs, ValidateServiceSubnet(c.Networking.ServiceSubnet, field.NewPath("service subnet"))...)
allErrs = append(allErrs, ValidateCloudProvider(c.CloudProvider, field.NewPath("cloudprovider"))...)
allErrs = append(allErrs, ValidateAuthorizationMode(c.AuthorizationMode, field.NewPath("authorization-mode"))...)
return allErrs
}

Expand Down Expand Up @@ -96,6 +98,13 @@ func ValidateTokenDiscovery(c *kubeadm.TokenDiscovery, fldPath *field.Path) fiel
return allErrs
}

func ValidateAuthorizationMode(authzMode string, fldPath *field.Path) field.ErrorList {
if !authzmodes.IsValidAuthorizationMode(authzMode) {
return field.ErrorList{field.Invalid(fldPath, nil, "invalid authorization mode")}
}
return field.ErrorList{}
}

func ValidateServiceSubnet(subnet string, fldPath *field.Path) field.ErrorList {
_, svcSubnet, err := net.ParseCIDR(subnet)
if err != nil {
Expand Down
40 changes: 40 additions & 0 deletions cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go
View file
Open in desktop
Expand Up @@ -46,6 +46,30 @@ func TestValidateTokenDiscovery(t *testing.T) {
}
}

func TestValidateAuthorizationMode(t *testing.T) {
var tests = []struct {
s string
f *field.Path
expected bool
}{
{"", nil, false},
{"rBAC", nil, false}, // not supported
{"not valid", nil, false}, // not supported
{"RBAC", nil, true}, // supported
{"Webhook", nil, true}, // supported
}
for _, rt := range tests {
actual := ValidateAuthorizationMode(rt.s, rt.f)
if (len(actual) == 0) != rt.expected {
t.Errorf(
"failed ValidateAuthorizationMode:\n\texpected: %t\n\t actual: %t",
rt.expected,
(len(actual) == 0),
)
}
}
}

func TestValidateServiceSubnet(t *testing.T) {
var tests = []struct {
s string
Expand Down Expand Up @@ -111,16 +135,28 @@ func TestValidateMasterConfiguration(t *testing.T) {
Addresses: []string{"foobar"},
},
},
AuthorizationMode: "RBAC",
Networking: kubeadm.Networking{
ServiceSubnet: "10.96.0.1/12",
},
}, false},
{&kubeadm.MasterConfiguration{
Discovery: kubeadm.Discovery{
HTTPS: &kubeadm.HTTPSDiscovery{URL: "foo"},
},
AuthorizationMode: "RBAC",
Networking: kubeadm.Networking{
ServiceSubnet: "10.96.0.1/12",
},
}, true},
{&kubeadm.MasterConfiguration{
Discovery: kubeadm.Discovery{
File: &kubeadm.FileDiscovery{Path: "foo"},
},
AuthorizationMode: "RBAC",
Networking: kubeadm.Networking{
ServiceSubnet: "10.96.0.1/12",
},
}, true},
{&kubeadm.MasterConfiguration{
Discovery: kubeadm.Discovery{
Expand All @@ -130,6 +166,10 @@ func TestValidateMasterConfiguration(t *testing.T) {
Addresses: []string{"foobar"},
},
},
AuthorizationMode: "RBAC",
Networking: kubeadm.Networking{
ServiceSubnet: "10.96.0.1/12",
},
}, true},
}
for _, rt := range tests {
Expand Down
8 changes: 3 additions & 5 deletions cmd/kubeadm/app/cmd/init.go
View file
Open in desktop
Expand Up @@ -253,11 +253,9 @@ func (i *Init) Run(out io.Writer) error {
return err
}

if i.cfg.AuthorizationMode == kubeadmconstants.AuthzModeRBAC {
err = apiconfigphase.CreateRBACRules(client)
if err != nil {
return err
}
err = apiconfigphase.CreateRBACRules(client)
if err != nil {
return err
}

if err := addonsphase.CreateEssentialAddons(i.cfg, client); err != nil {
Expand Down
22 changes: 11 additions & 11 deletions cmd/kubeadm/app/constants/constants.go
View file
Open in desktop
Expand Up @@ -16,11 +16,14 @@ limitations under the License.

package constants

import "time"
import (
"path"
"time"
)

const (
AuthorizationPolicyFile = "abac_policy.json"
AuthorizationWebhookConfigFile = "webhook_authz.conf"
// KubernetesDir is the directory kubernetes owns for storing various configuration files
KubernetesDir = "/etc/kubernetes"
Copy link
Member

@liggitt liggitt Feb 21, 2017 •
edited

Choose a reason for hiding this comment

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

not sure about this as a fixed, non-overrideable path

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a fine line between overridability and sprawl. Regarding this one, we decided in kubernetes/kubeadm#156 that /etc/kubernetes should be a constant -- it doesn't make sense to override that really.
Also, this isn't new for this PR. If you have very strong opinions -- please file an issue and we'll see but it's not the main thing in this PR


CACertAndKeyBaseName = "ca"
CACertName = "ca.crt"
Expand Down Expand Up @@ -49,14 +52,6 @@ const (
AdminKubeConfigFileName = "admin.conf"
KubeletKubeConfigFileName = "kubelet.conf"

// TODO: These constants should actually come from pkg/kubeapiserver/authorizer, but we can't vendor that package in now
// because of all the other sub-packages that would get vendored. To fix this, a pkg/kubeapiserver/authorizer/modes package
// or similar should exist that only has these constants; then we can vendor it.
AuthzModeAlwaysAllow = "AlwaysAllow"
AuthzModeABAC = "ABAC"
AuthzModeRBAC = "RBAC"
AuthzModeWebhook = "Webhook"

// Important: a "v"-prefix shouldn't exist here; semver doesn't allow that
MinimumControlPlaneVersion = "1.6.0-alpha.2"

Expand All @@ -83,3 +78,8 @@ const (
// The file name of the tokens file that can be used for bootstrapping
CSVTokenFileName = "tokens.csv"
)

var (
AuthorizationPolicyPath = path.Join(KubernetesDir, "abac_policy.json")
AuthorizationWebhookConfigPath = path.Join(KubernetesDir, "webhook_authz.conf")
Copy link
Member

Choose a reason for hiding this comment

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

is the switch to a hard-coded absolute path new? seems like that would make testing difficult

Copy link
Member Author

Choose a reason for hiding this comment

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

It isn't new. We decided on making these params hard coded -- at least until we have the full config file backend to rely on where the user can specify params that are passed directly to the apiserver/cm/scheduler

)
1 change: 1 addition & 0 deletions cmd/kubeadm/app/master/BUILD
View file
Open in desktop
Expand Up @@ -24,6 +24,7 @@ go_library(
"//cmd/kubeadm/app/images:go_default_library",
"//cmd/kubeadm/app/util:go_default_library",
"//cmd/kubeadm/app/util/kubeconfig:go_default_library",
"//pkg/kubeapiserver/authorizer/modes:go_default_library",
"//pkg/kubectl/cmd/util:go_default_library",
"//vendor:github.com/ghodss/yaml",
"//vendor:k8s.io/apimachinery/pkg/api/errors",
Expand Down
30 changes: 21 additions & 9 deletions cmd/kubeadm/app/master/manifests.go
View file
Open in desktop
Expand Up @@ -32,6 +32,7 @@ import (
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
"k8s.io/kubernetes/cmd/kubeadm/app/images"
authzmodes "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
)

Expand Down Expand Up @@ -326,15 +327,7 @@ func getAPIServerCommand(cfg *kubeadmapi.MasterConfiguration, selfHosted bool) [
"--requestheader-allowed-names=front-proxy-client",
)

if cfg.AuthorizationMode != "" {
command = append(command, "--authorization-mode="+cfg.AuthorizationMode)
switch cfg.AuthorizationMode {
case kubeadmconstants.AuthzModeABAC:
command = append(command, "--authorization-policy-file="+path.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.AuthorizationPolicyFile))
case kubeadmconstants.AuthzModeWebhook:
command = append(command, "--authorization-webhook-config-file="+path.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.AuthorizationWebhookConfigFile))
}
}
command = append(command, getAuthzParameters(cfg.AuthorizationMode)...)

// Use first address we are given
if len(cfg.API.AdvertiseAddresses) > 0 {
Expand Down Expand Up @@ -459,3 +452,22 @@ func getSelfHostedAPIServerEnv() []api.EnvVar {

return append(getProxyEnvVars(), podIPEnvVar)
}

func getAuthzParameters(authzMode string) []string {
command := []string{}
// RBAC is always on. If the user specified
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment doesn't parse for me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops. This is artwork as a result of push notifications I guess 😄
I'll fix that up in a follow up -- I have lots of other PRs coming

authzModes := []string{authzmodes.ModeRBAC}
if len(authzMode) != 0 && authzMode != authzmodes.ModeRBAC {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to interpret authzModes as a list similar to the way the apiserver does? That would change this code quite a bit. But it might be good to mirror what the apiserver does.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, that might make sense. However, if we do decide so (please file an issue and ping me), that's for an other PR to avoid doing too many things in one.

authzModes = append(authzModes, authzMode)
}

command = append(command, "--authorization-mode="+strings.Join(authzModes, ","))

switch authzMode {
case authzmodes.ModeABAC:
command = append(command, "--authorization-policy-file="+kubeadmconstants.AuthorizationPolicyPath)
case authzmodes.ModeWebhook:
command = append(command, "--authorization-webhook-config-file="+kubeadmconstants.AuthorizationWebhookConfigPath)
}
return command
}
62 changes: 62 additions & 0 deletions cmd/kubeadm/app/master/manifests_test.go
View file
Open in desktop
Expand Up @@ -388,6 +388,7 @@ func TestGetAPIServerCommand(t *testing.T) {
"--requestheader-extra-headers-prefix=X-Remote-Extra-",
"--requestheader-client-ca-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/front-proxy-ca.crt",
"--requestheader-allowed-names=front-proxy-client",
"--authorization-mode=RBAC",
"--etcd-servers=http://127.0.0.1:2379",
},
},
Expand Down Expand Up @@ -417,6 +418,7 @@ func TestGetAPIServerCommand(t *testing.T) {
"--requestheader-extra-headers-prefix=X-Remote-Extra-",
"--requestheader-client-ca-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/front-proxy-ca.crt",
"--requestheader-allowed-names=front-proxy-client",
"--authorization-mode=RBAC",
"--advertise-address=foo",
"--etcd-servers=http://127.0.0.1:2379",
},
Expand Down Expand Up @@ -448,6 +450,7 @@ func TestGetAPIServerCommand(t *testing.T) {
"--requestheader-extra-headers-prefix=X-Remote-Extra-",
"--requestheader-client-ca-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/front-proxy-ca.crt",
"--requestheader-allowed-names=front-proxy-client",
"--authorization-mode=RBAC",
"--etcd-servers=http://127.0.0.1:2379",
"--etcd-certfile=fiz",
"--etcd-keyfile=faz",
Expand Down Expand Up @@ -567,3 +570,62 @@ func TestGetSchedulerCommand(t *testing.T) {
}
}
}

func TestGetAuthzParameters(t *testing.T) {
var tests = []struct {
authMode string
expected []string
}{
{
authMode: "",
expected: []string{
"--authorization-mode=RBAC",
},
},
{
authMode: "RBAC",
expected: []string{
"--authorization-mode=RBAC",
},
},
{
authMode: "AlwaysAllow",
expected: []string{
"--authorization-mode=RBAC,AlwaysAllow",
},
},
{
authMode: "AlwaysDeny",
expected: []string{
"--authorization-mode=RBAC,AlwaysDeny",
},
},
{
authMode: "ABAC",
expected: []string{
"--authorization-mode=RBAC,ABAC",
"--authorization-policy-file=/etc/kubernetes/abac_policy.json",
},
},
{
authMode: "Webhook",
expected: []string{
"--authorization-mode=RBAC,Webhook",
"--authorization-webhook-config-file=/etc/kubernetes/webhook_authz.conf",
},
},
}

for _, rt := range tests {
actual := getAuthzParameters(rt.authMode)
for i := range actual {
if actual[i] != rt.expected[i] {
t.Errorf(
"failed getAuthzParameters:\n\texpected: %s\n\t actual: %s",
rt.expected[i],
actual[i],
)
}
}
}
}
1 change: 1 addition & 0 deletions cmd/kubeadm/app/preflight/BUILD
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@ go_library(
"//cmd/kubeadm/app/apis/kubeadm:go_default_library",
"//cmd/kubeadm/app/constants:go_default_library",
"//pkg/api/validation:go_default_library",
"//pkg/kubeapiserver/authorizer/modes:go_default_library",
"//pkg/util/initsystem:go_default_library",
"//pkg/util/node:go_default_library",
"//test/e2e_node/system:go_default_library",
Expand Down
11 changes: 5 additions & 6 deletions cmd/kubeadm/app/preflight/checks.go
View file
Open in desktop
Expand Up @@ -31,6 +31,7 @@ import (
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
"k8s.io/kubernetes/pkg/api/validation"
authzmodes "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
"k8s.io/kubernetes/pkg/util/initsystem"
"k8s.io/kubernetes/pkg/util/node"
"k8s.io/kubernetes/test/e2e_node/system"
Expand Down Expand Up @@ -363,12 +364,10 @@ func RunInitMasterChecks(cfg *kubeadmapi.MasterConfiguration) error {

// Check the config for authorization mode
switch cfg.AuthorizationMode {
case kubeadmconstants.AuthzModeABAC:
authorizationPolicyPath := filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.AuthorizationPolicyFile)
checks = append(checks, FileExistingCheck{Path: authorizationPolicyPath})
case kubeadmconstants.AuthzModeWebhook:
authorizationWebhookConfigPath := filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.AuthorizationWebhookConfigFile)
checks = append(checks, FileExistingCheck{Path: authorizationWebhookConfigPath})
case authzmodes.ModeABAC:
checks = append(checks, FileExistingCheck{Path: kubeadmconstants.AuthorizationPolicyPath})
case authzmodes.ModeWebhook:
checks = append(checks, FileExistingCheck{Path: kubeadmconstants.AuthorizationWebhookConfigPath})
}

return RunChecks(checks, os.Stderr)
Expand Down
7 changes: 6 additions & 1 deletion pkg/kubeapiserver/authorizer/BUILD
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@ go_test(
],
library = ":go_default_library",
tags = ["automanaged"],
deps = ["//pkg/kubeapiserver/authorizer/modes:go_default_library"],
)

go_library(
Expand All @@ -25,6 +26,7 @@ go_library(
deps = [
"//pkg/auth/authorizer/abac:go_default_library",
"//pkg/controller/informers:go_default_library",
"//pkg/kubeapiserver/authorizer/modes:go_default_library",
"//plugin/pkg/auth/authorizer/rbac:go_default_library",
"//vendor:k8s.io/apiserver/pkg/authorization/authorizer",
"//vendor:k8s.io/apiserver/pkg/authorization/authorizerfactory",
Expand All @@ -42,6 +44,9 @@ filegroup(

filegroup(
name = "all-srcs",
srcs = [":package-srcs"],
srcs = [
":package-srcs",
"//pkg/kubeapiserver/authorizer/modes:all-srcs",
],
tags = ["automanaged"],
)