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

migrate group approver to use subject access reviews #45619

Merged
merged 2 commits into from
May 31, 2017
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions cmd/kube-controller-manager/app/certificates.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,11 @@ func startCSRApprovingController(ctx ControllerContext) (bool, error) {
if !ctx.AvailableResources[schema.GroupVersionResource{Group: "certificates.k8s.io", Version: "v1beta1", Resource: "certificatesigningrequests"}] {
return false, nil
}
if ctx.Options.ApproveAllKubeletCSRsForGroup == "" {
return false, nil
}
c := ctx.ClientBuilder.ClientOrDie("certificate-controller")

approver, err := approver.NewCSRApprovingController(
c,
ctx.InformerFactory.Certificates().V1beta1().CertificateSigningRequests(),
ctx.Options.ApproveAllKubeletCSRsForGroup,
)
if err != nil {
// TODO this is failing consistently in test-cmd and local-up-cluster.sh. Fix them and make it consistent with all others which
Expand Down
4 changes: 3 additions & 1 deletion cmd/kube-controller-manager/app/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,9 @@ func (s *CMServer) AddFlags(fs *pflag.FlagSet, allControllers []string, disabled
fs.StringVar(&s.ClusterSigningCertFile, "cluster-signing-cert-file", s.ClusterSigningCertFile, "Filename containing a PEM-encoded X509 CA certificate used to issue cluster-scoped certificates")
fs.StringVar(&s.ClusterSigningKeyFile, "cluster-signing-key-file", s.ClusterSigningKeyFile, "Filename containing a PEM-encoded RSA or ECDSA private key used to sign cluster-scoped certificates")
fs.DurationVar(&s.ClusterSigningDuration.Duration, "experimental-cluster-signing-duration", s.ClusterSigningDuration.Duration, "The length of duration signed certificates will be given.")
fs.StringVar(&s.ApproveAllKubeletCSRsForGroup, "insecure-experimental-approve-all-kubelet-csrs-for-group", s.ApproveAllKubeletCSRsForGroup, "The group for which the controller-manager will auto approve all CSRs for kubelet client certificates.")
var dummy string
fs.MarkDeprecated("insecure-experimental-approve-all-kubelet-csrs-for-group", "This flag does nothing.")
fs.StringVar(&dummy, "insecure-experimental-approve-all-kubelet-csrs-for-group", "", "This flag does nothing.")
fs.BoolVar(&s.EnableProfiling, "profiling", true, "Enable profiling via web interface host:port/debug/pprof/")
fs.BoolVar(&s.EnableContentionProfiling, "contention-profiling", false, "Enable lock contention profiling, if profiling is enabled")
fs.StringVar(&s.ClusterName, "cluster-name", s.ClusterName, "The instance prefix for the cluster")
Expand Down
1 change: 0 additions & 1 deletion cmd/kubeadm/app/master/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ go_library(
"//cmd/kubeadm/app/constants:go_default_library",
"//cmd/kubeadm/app/images:go_default_library",
"//cmd/kubeadm/app/util/kubeconfig:go_default_library",
"//pkg/bootstrap/api:go_default_library",
"//pkg/kubeapiserver/authorizer/modes:go_default_library",
"//pkg/kubectl/cmd/util:go_default_library",
"//pkg/util/version:go_default_library",
Expand Down
20 changes: 9 additions & 11 deletions cmd/kubeadm/app/master/manifests.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import (
kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
"k8s.io/kubernetes/cmd/kubeadm/app/images"
bootstrapapi "k8s.io/kubernetes/pkg/bootstrap/api"
authzmodes "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/util/version"
Expand Down Expand Up @@ -419,16 +418,15 @@ func getControllerManagerCommand(cfg *kubeadmapi.MasterConfiguration, selfHosted
}

defaultArguments := map[string]string{
"address": "127.0.0.1",
"leader-elect": "true",
"kubeconfig": filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.ControllerManagerKubeConfigFileName),
"root-ca-file": filepath.Join(cfg.CertificatesDir, kubeadmconstants.CACertName),
"service-account-private-key-file": filepath.Join(cfg.CertificatesDir, kubeadmconstants.ServiceAccountPrivateKeyName),
"cluster-signing-cert-file": filepath.Join(cfg.CertificatesDir, kubeadmconstants.CACertName),
"cluster-signing-key-file": filepath.Join(cfg.CertificatesDir, kubeadmconstants.CAKeyName),
"insecure-experimental-approve-all-kubelet-csrs-for-group": bootstrapapi.BootstrapGroup,
"use-service-account-credentials": "true",
"controllers": "*,bootstrapsigner,tokencleaner",
"address": "127.0.0.1",
"leader-elect": "true",
"kubeconfig": filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.ControllerManagerKubeConfigFileName),
"root-ca-file": filepath.Join(cfg.CertificatesDir, kubeadmconstants.CACertName),
"service-account-private-key-file": filepath.Join(cfg.CertificatesDir, kubeadmconstants.ServiceAccountPrivateKeyName),
"cluster-signing-cert-file": filepath.Join(cfg.CertificatesDir, kubeadmconstants.CACertName),
"cluster-signing-key-file": filepath.Join(cfg.CertificatesDir, kubeadmconstants.CAKeyName),
"use-service-account-credentials": "true",
"controllers": "*,bootstrapsigner,tokencleaner",
}

command = getComponentBaseCommand(controllerManager)
Expand Down
3 changes: 0 additions & 3 deletions cmd/kubeadm/app/master/manifests_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,6 @@ func TestGetControllerManagerCommand(t *testing.T) {
"--service-account-private-key-file=" + testCertsDir + "/sa.key",
"--cluster-signing-cert-file=" + testCertsDir + "/ca.crt",
"--cluster-signing-key-file=" + testCertsDir + "/ca.key",
"--insecure-experimental-approve-all-kubelet-csrs-for-group=system:bootstrappers",
"--use-service-account-credentials=true",
"--controllers=*,bootstrapsigner,tokencleaner",
},
Expand All @@ -695,7 +694,6 @@ func TestGetControllerManagerCommand(t *testing.T) {
"--service-account-private-key-file=" + testCertsDir + "/sa.key",
"--cluster-signing-cert-file=" + testCertsDir + "/ca.crt",
"--cluster-signing-key-file=" + testCertsDir + "/ca.key",
"--insecure-experimental-approve-all-kubelet-csrs-for-group=system:bootstrappers",
"--use-service-account-credentials=true",
"--controllers=*,bootstrapsigner,tokencleaner",
"--cloud-provider=foo",
Expand All @@ -715,7 +713,6 @@ func TestGetControllerManagerCommand(t *testing.T) {
"--service-account-private-key-file=" + testCertsDir + "/sa.key",
"--cluster-signing-cert-file=" + testCertsDir + "/ca.crt",
"--cluster-signing-key-file=" + testCertsDir + "/ca.key",
"--insecure-experimental-approve-all-kubelet-csrs-for-group=system:bootstrappers",
"--use-service-account-credentials=true",
"--controllers=*,bootstrapsigner,tokencleaner",
"--allocate-node-cidrs=true",
Expand Down
92 changes: 64 additions & 28 deletions cmd/kubeadm/app/phases/apiconfig/clusterroles.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,32 +35,16 @@ const (
// BootstrapSignerClusterRoleName sets the name for the ClusterRole that allows access to ConfigMaps in the kube-public ns
BootstrapSignerClusterRoleName = "system:bootstrap-signer-clusterinfo"

// Constants
clusterRoleKind = "ClusterRole"
roleKind = "Role"
serviceAccountKind = "ServiceAccount"
rbacAPIGroup = "rbac.authorization.k8s.io"
anonymousUser = "system:anonymous"
clusterRoleKind = "ClusterRole"
roleKind = "Role"
serviceAccountKind = "ServiceAccount"
rbacAPIGroup = "rbac.authorization.k8s.io"
anonymousUser = "system:anonymous"
nodeAutoApproveBootstrap = "kubeadm:node-autoapprove-bootstrap"
)

// TODO: Are there any unit tests that could be made for this file other than duplicating all values and logic in a separate file?

// CreateRBACRules creates the essential RBAC rules for a minimally set-up cluster
func CreateRBACRules(clientset *clientset.Clientset) error {
if err := CreateRoles(clientset); err != nil {
return err
}
if err := CreateRoleBindings(clientset); err != nil {
return err
}
if err := CreateClusterRoleBindings(clientset); err != nil {
return err
}

fmt.Println("[apiconfig] Created RBAC rules")
return nil
}

// CreateServiceAccounts creates the necessary serviceaccounts that kubeadm uses/might use.
func CreateServiceAccounts(clientset *clientset.Clientset) error {
serviceAccounts := []v1.ServiceAccount{
Expand All @@ -86,8 +70,26 @@ func CreateServiceAccounts(clientset *clientset.Clientset) error {
return nil
}

// CreateRoles creates namespaces RBAC Roles
func CreateRoles(clientset *clientset.Clientset) error {
// CreateRBACRules creates the essential RBAC rules for a minimally set-up cluster
func CreateRBACRules(clientset *clientset.Clientset) error {
if err := createRoles(clientset); err != nil {
return err
}
if err := createRoleBindings(clientset); err != nil {
return err
}
if err := createClusterRoles(clientset); err != nil {
return err
}
if err := createClusterRoleBindings(clientset); err != nil {
return err
}

fmt.Println("[apiconfig] Created RBAC rules")
return nil
}

func createRoles(clientset *clientset.Clientset) error {
roles := []rbac.Role{
{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -107,8 +109,7 @@ func CreateRoles(clientset *clientset.Clientset) error {
return nil
}

// CreateRoleBindings creates all namespaced and necessary bindings between bootstrapped & kubeadm-created ClusterRoles and subjects kubeadm is using
func CreateRoleBindings(clientset *clientset.Clientset) error {
func createRoleBindings(clientset *clientset.Clientset) error {
roleBindings := []rbac.RoleBinding{
{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -137,8 +138,27 @@ func CreateRoleBindings(clientset *clientset.Clientset) error {
return nil
}

// CreateClusterRoleBindings creates all necessary bindings between bootstrapped & kubeadm-created ClusterRoles and subjects kubeadm is using
func CreateClusterRoleBindings(clientset *clientset.Clientset) error {
func createClusterRoles(clientset *clientset.Clientset) error {
clusterRoles := []rbac.ClusterRole{
{
ObjectMeta: metav1.ObjectMeta{
Name: nodeAutoApproveBootstrap,
},
Rules: []rbac.PolicyRule{
rbac.NewRule("create").Groups("certificates.k8s.io").Resources("certificatesigningrequests/nodeclient").RuleOrDie(),
Copy link
Member

Choose a reason for hiding this comment

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

constant reference for the resource/subresource would be ideal

Copy link
Member

Choose a reason for hiding this comment

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

cc @cjcullen for CSR subresource authorization checks

},
},
}

for _, roleBinding := range clusterRoles {
if _, err := clientset.RbacV1beta1().ClusterRoles().Create(&roleBinding); err != nil {
return err
}
}
return nil
}

func createClusterRoleBindings(clientset *clientset.Clientset) error {
clusterRoleBindings := []rbac.ClusterRoleBinding{
{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -156,6 +176,22 @@ func CreateClusterRoleBindings(clientset *clientset.Clientset) error {
},
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: nodeAutoApproveBootstrap,
},
RoleRef: rbac.RoleRef{
APIGroup: rbacAPIGroup,
Kind: clusterRoleKind,
Name: nodeAutoApproveBootstrap,
},
Subjects: []rbac.Subject{
{
Kind: "Group",
Name: bootstrapapi.BootstrapGroup,
},
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "kubeadm:node-proxier",
Expand Down
6 changes: 0 additions & 6 deletions pkg/apis/componentconfig/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -839,12 +839,6 @@ type KubeControllerManagerConfiguration struct {
// clusterSigningDuration is the length of duration signed certificates
// will be given.
ClusterSigningDuration metav1.Duration
// approveAllKubeletCSRs tells the CSR controller to approve all CSRs originating
// from the kubelet bootstrapping group automatically.
// WARNING: this grants all users with access to the certificates API group
// the ability to create credentials for any user that has access to the boostrapping
// user's credentials.
ApproveAllKubeletCSRsForGroup string
// enableProfiling enables profiling via web interface host:port/debug/pprof/
EnableProfiling bool
// enableContentionProfiling enables lock contention profiling, if enableProfiling is true.
Expand Down
15 changes: 11 additions & 4 deletions pkg/controller/certificates/approver/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,29 @@ load(

go_test(
name = "go_default_test",
srcs = ["groupapprove_test.go"],
srcs = ["sarapprove_test.go"],
library = ":go_default_library",
tags = ["automanaged"],
deps = ["//pkg/apis/certificates/v1beta1:go_default_library"],
deps = [
"//pkg/apis/authorization/v1beta1:go_default_library",
"//pkg/apis/certificates/v1beta1:go_default_library",
"//pkg/client/clientset_generated/clientset/fake:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/k8s.io/client-go/testing:go_default_library",
],
)

go_library(
name = "go_default_library",
srcs = ["groupapprove.go"],
srcs = ["sarapprove.go"],
tags = ["automanaged"],
deps = [
"//pkg/apis/authorization/v1beta1:go_default_library",
"//pkg/apis/certificates/v1beta1:go_default_library",
"//pkg/client/clientset_generated/clientset:go_default_library",
"//pkg/client/informers/informers_generated/externalversions/certificates/v1beta1:go_default_library",
"//pkg/controller/certificates:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
],
)

Expand Down