From 4f969bdfc2a896d8a1f905d738c0c634281f0804 Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Wed, 25 Jan 2017 17:01:48 -0800 Subject: [PATCH 1/7] Add helpers for creating an RBAC RoleBinding. --- pkg/apis/rbac/helpers.go | 60 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/pkg/apis/rbac/helpers.go b/pkg/apis/rbac/helpers.go index 55875a114ca..e04c22cbbf9 100644 --- a/pkg/apis/rbac/helpers.go +++ b/pkg/apis/rbac/helpers.go @@ -254,3 +254,63 @@ func (r *ClusterRoleBindingBuilder) Binding() (ClusterRoleBinding, error) { return r.ClusterRoleBinding, nil } + +// +k8s:deepcopy-gen=false +// RoleBindingBuilder let's us attach methods. It is similar to +// ClusterRoleBindingBuilder above. +type RoleBindingBuilder struct { + RoleBinding RoleBinding +} + +func NewRoleBinding(roleName, namespace string) *RoleBindingBuilder { + return &RoleBindingBuilder{ + RoleBinding: RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: roleName, + Namespace: namespace, + }, + RoleRef: RoleRef{ + APIGroup: GroupName, + Kind: "Role", + Name: roleName, + }, + }, + } +} + +func (r *RoleBindingBuilder) Groups(groups ...string) *RoleBindingBuilder { + for _, group := range groups { + r.RoleBinding.Subjects = append(r.RoleBinding.Subjects, Subject{Kind: GroupKind, Name: group}) + } + return r +} + +func (r *RoleBindingBuilder) Users(users ...string) *RoleBindingBuilder { + for _, user := range users { + r.RoleBinding.Subjects = append(r.RoleBinding.Subjects, Subject{Kind: UserKind, Name: user}) + } + return r +} + +func (r *RoleBindingBuilder) SAs(namespace string, serviceAccountNames ...string) *RoleBindingBuilder { + for _, saName := range serviceAccountNames { + r.RoleBinding.Subjects = append(r.RoleBinding.Subjects, Subject{Kind: ServiceAccountKind, Namespace: namespace, Name: saName}) + } + return r +} + +func (r *RoleBindingBuilder) BindingOrDie() RoleBinding { + ret, err := r.Binding() + if err != nil { + panic(err) + } + return ret +} + +func (r *RoleBindingBuilder) Binding() (RoleBinding, error) { + if len(r.RoleBinding.Subjects) == 0 { + return RoleBinding{}, fmt.Errorf("subjects are required: %#v", r.RoleBinding) + } + + return r.RoleBinding, nil +} From 1bb80bca0828a4fc368e9e4afb2e155dc5319331 Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Wed, 25 Jan 2017 17:02:48 -0800 Subject: [PATCH 2/7] [Federation][kubefed] Create a dedicated service account for federation controller manager in the host cluster and give it appropriate permissions. --- federation/pkg/kubefed/init/init.go | 71 +++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 3 deletions(-) diff --git a/federation/pkg/kubefed/init/init.go b/federation/pkg/kubefed/init/init.go index b0704c5b224..680aceeda1d 100644 --- a/federation/pkg/kubefed/init/init.go +++ b/federation/pkg/kubefed/init/init.go @@ -47,6 +47,7 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/resource" "k8s.io/kubernetes/pkg/apis/extensions" + "k8s.io/kubernetes/pkg/apis/rbac" client "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" @@ -62,6 +63,16 @@ const ( AdminCN = "admin" HostClusterLocalDNSZoneName = "cluster.local." + // User name used by federation controller manager to make + // calls to federation API server. + ControllerManagerUser = "federation-controller-manager" + + // Name of the ServiceAccount used by the federation controller manager. + ControllerManagerSA = "federation-controller-manager" + + // Group name of the legacy/core API group + legacyAPIGroup = "" + lbAddrRetryInterval = 5 * time.Second podWaitInterval = 2 * time.Second ) @@ -220,7 +231,21 @@ func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Comman } // 7. Create federation controller manager - _, err = createControllerManager(hostClientset, initFlags.FederationSystemNamespace, initFlags.Name, svc.Name, cmName, image, cmKubeconfigName, dnsZoneName, dnsProvider, dryRun) + // 7a. Create service accounts for federation controller manager + sa, err := createControllerManagerSA(hostClientset, initFlags.FederationSystemNamespace, dryRun) + if err != nil { + return err + } + + // 7b. Create RBAC role and role binding for federation controller + // manager service account. + _, _, err = createRoleBindings(hostClientset, initFlags.FederationSystemNamespace, sa.Name, dryRun) + if err != nil { + return err + } + + // 7c. Create federation controller manager deployment. + _, err = createControllerManager(hostClientset, initFlags.FederationSystemNamespace, initFlags.Name, svc.Name, cmName, image, cmKubeconfigName, dnsZoneName, dnsProvider, sa.Name, dryRun) if err != nil { return err } @@ -375,7 +400,7 @@ func createControllerManagerKubeconfigSecret(clientset *client.Clientset, namesp config := kubeadmkubeconfigphase.MakeClientConfigWithCerts( fmt.Sprintf("https://%s", svcName), name, - "federation-controller-manager", + ControllerManagerUser, certutil.EncodeCertPEM(entKeyPairs.ca.Cert), certutil.EncodePrivateKeyPEM(entKeyPairs.controllerManager.Key), certutil.EncodeCertPEM(entKeyPairs.controllerManager.Cert), @@ -520,7 +545,46 @@ func createAPIServer(clientset *client.Clientset, namespace, name, image, creden return clientset.Extensions().Deployments(namespace).Create(dep) } -func createControllerManager(clientset *client.Clientset, namespace, name, svcName, cmName, image, kubeconfigName, dnsZoneName, dnsProvider string, dryRun bool) (*extensions.Deployment, error) { +func createControllerManagerSA(clientset *client.Clientset, namespace string, dryRun bool) (*api.ServiceAccount, error) { + sa := &api.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: ControllerManagerSA, + Namespace: namespace, + }, + } + if dryRun { + return sa, nil + } + return clientset.Core().ServiceAccounts(namespace).Create(sa) +} + +func createRoleBindings(clientset *client.Clientset, namespace, saName string, dryRun bool) (*rbac.Role, *rbac.RoleBinding, error) { + roleName := "federation-system:federation-controller-manager" + role := &rbac.Role{ + // a role to use for bootstrapping the federation-controller-manager so it can access + // secrets in the host cluster to access other clusters. + ObjectMeta: metav1.ObjectMeta{ + Name: roleName, + }, + Rules: []rbac.PolicyRule{ + rbac.NewRule("get", "list", "watch").Groups(legacyAPIGroup).Resources("secrets").RuleOrDie(), + }, + } + rolebinding := rbac.NewRoleBinding(roleName, namespace).SAs(namespace, saName).BindingOrDie() + + if dryRun { + newRole, err := clientset.Rbac().Roles(namespace).Create(role) + if err != nil { + return nil, nil, err + } + + newRolebinding, err := clientset.Rbac().RoleBindings(namespace).Create(&rolebinding) + return newRole, newRolebinding, err + } + return role, &rolebinding, nil +} + +func createControllerManager(clientset *client.Clientset, namespace, name, svcName, cmName, image, kubeconfigName, dnsZoneName, dnsProvider, saName string, dryRun bool) (*extensions.Deployment, error) { dep := &extensions.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: cmName, @@ -578,6 +642,7 @@ func createControllerManager(clientset *client.Clientset, namespace, name, svcNa }, }, }, + ServiceAccountName: saName, }, }, }, From f52196397f7988b2d2a2693424a82fd86153a2f0 Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Thu, 26 Jan 2017 20:23:46 -0800 Subject: [PATCH 3/7] Added labels to the newly created objects. --- federation/pkg/kubefed/init/init.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/federation/pkg/kubefed/init/init.go b/federation/pkg/kubefed/init/init.go index 680aceeda1d..30fe14a1769 100644 --- a/federation/pkg/kubefed/init/init.go +++ b/federation/pkg/kubefed/init/init.go @@ -550,6 +550,7 @@ func createControllerManagerSA(clientset *client.Clientset, namespace string, dr ObjectMeta: metav1.ObjectMeta{ Name: ControllerManagerSA, Namespace: namespace, + Labels: componentLabel, }, } if dryRun { @@ -564,24 +565,30 @@ func createRoleBindings(clientset *client.Clientset, namespace, saName string, d // a role to use for bootstrapping the federation-controller-manager so it can access // secrets in the host cluster to access other clusters. ObjectMeta: metav1.ObjectMeta{ - Name: roleName, + Name: roleName, + Namespace: namespace, + Labels: componentLabel, }, Rules: []rbac.PolicyRule{ rbac.NewRule("get", "list", "watch").Groups(legacyAPIGroup).Resources("secrets").RuleOrDie(), }, } + rolebinding := rbac.NewRoleBinding(roleName, namespace).SAs(namespace, saName).BindingOrDie() + rolebinding.Namespace = namespace + rolebinding.Labels = componentLabel if dryRun { - newRole, err := clientset.Rbac().Roles(namespace).Create(role) - if err != nil { - return nil, nil, err - } - - newRolebinding, err := clientset.Rbac().RoleBindings(namespace).Create(&rolebinding) - return newRole, newRolebinding, err + return role, &rolebinding, nil } - return role, &rolebinding, nil + + newRole, err := clientset.Rbac().Roles(namespace).Create(role) + if err != nil { + return nil, nil, err + } + + newRolebinding, err := clientset.Rbac().RoleBindings(namespace).Create(&rolebinding) + return newRole, newRolebinding, err } func createControllerManager(clientset *client.Clientset, namespace, name, svcName, cmName, image, kubeconfigName, dnsZoneName, dnsProvider, saName string, dryRun bool) (*extensions.Deployment, error) { From 42ff4e354c90a11a1f03538274c5dadd4052d9ee Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Thu, 26 Jan 2017 20:23:59 -0800 Subject: [PATCH 4/7] Add unit tests. --- federation/pkg/kubefed/init/init_test.go | 102 +++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/federation/pkg/kubefed/init/init_test.go b/federation/pkg/kubefed/init/init_test.go index 21e7961d66b..2acb8cc610a 100644 --- a/federation/pkg/kubefed/init/init_test.go +++ b/federation/pkg/kubefed/init/init_test.go @@ -43,6 +43,7 @@ import ( "k8s.io/kubernetes/pkg/api/testapi" "k8s.io/kubernetes/pkg/api/v1" "k8s.io/kubernetes/pkg/apis/extensions/v1beta1" + rbacv1beta1 "k8s.io/kubernetes/pkg/apis/rbac/v1beta1" cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/util/intstr" @@ -554,6 +555,62 @@ func fakeInitHostFactory(federationName, namespaceName, ip, dnsZoneName, image, }, } + sa := v1.ServiceAccount{ + TypeMeta: metav1.TypeMeta{ + Kind: "ServiceAccount", + APIVersion: testapi.Default.GroupVersion().String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "federation-controller-manager", + Namespace: namespaceName, + Labels: componentLabel, + }, + } + + role := rbacv1beta1.Role{ + TypeMeta: metav1.TypeMeta{ + Kind: "Role", + APIVersion: testapi.Rbac.GroupVersion().String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "federation-system:federation-controller-manager", + Namespace: namespaceName, + Labels: componentLabel, + }, + Rules: []rbacv1beta1.PolicyRule{ + { + Verbs: []string{"get", "list", "watch"}, + APIGroups: []string{""}, + Resources: []string{"secrets"}, + }, + }, + } + + rolebinding := rbacv1beta1.RoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "RoleBinding", + APIVersion: testapi.Rbac.GroupVersion().String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "federation-system:federation-controller-manager", + Namespace: namespaceName, + Labels: componentLabel, + }, + Subjects: []rbacv1beta1.Subject{ + { + Kind: "ServiceAccount", + APIVersion: "", + Name: "federation-controller-manager", + Namespace: "federation-system", + }, + }, + RoleRef: rbacv1beta1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "Role", + Name: "federation-system:federation-controller-manager", + }, + } + apiserver := v1beta1.Deployment{ TypeMeta: metav1.TypeMeta{ Kind: "Deployment", @@ -709,6 +766,8 @@ func fakeInitHostFactory(federationName, namespaceName, ip, dnsZoneName, image, }, }, }, + ServiceAccountName: "federation-controller-manager", + DeprecatedServiceAccount: "federation-controller-manager", }, }, }, @@ -748,6 +807,7 @@ func fakeInitHostFactory(federationName, namespaceName, ip, dnsZoneName, image, f, tf, codec, _ := cmdtesting.NewAPIFactory() extCodec := testapi.Extensions.Codec() + rbacCodec := testapi.Rbac.Codec() ns := dynamic.ContentConfig().NegotiatedSerializer tf.ClientConfig = kubefedtesting.DefaultClientConfig() tf.Client = &fake.RESTClient{ @@ -852,6 +912,48 @@ func fakeInitHostFactory(federationName, namespaceName, ip, dnsZoneName, image, return &http.Response{StatusCode: http.StatusCreated, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(extCodec, &want)}, nil case p == "/api/v1/namespaces/federation-system/pods" && m == http.MethodGet: return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &podList)}, nil + case p == "/api/v1/namespaces/federation-system/serviceaccounts" && m == http.MethodPost: + body, err := ioutil.ReadAll(req.Body) + if err != nil { + return nil, err + } + var got v1.ServiceAccount + _, _, err = codec.Decode(body, nil, &got) + if err != nil { + return nil, err + } + if !api.Semantic.DeepEqual(got, sa) { + return nil, fmt.Errorf("Unexpected service account object\n\tDiff: %s", diff.ObjectGoPrintDiff(got, sa)) + } + return &http.Response{StatusCode: http.StatusCreated, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &sa)}, nil + case p == "/apis/rbac.authorization.k8s.io/v1beta1/namespaces/federation-system/roles" && m == http.MethodPost: + body, err := ioutil.ReadAll(req.Body) + if err != nil { + return nil, err + } + var got rbacv1beta1.Role + _, _, err = codec.Decode(body, nil, &got) + if err != nil { + return nil, err + } + if !api.Semantic.DeepEqual(got, role) { + return nil, fmt.Errorf("Unexpected role object\n\tDiff: %s", diff.ObjectGoPrintDiff(got, role)) + } + return &http.Response{StatusCode: http.StatusCreated, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(rbacCodec, &role)}, nil + case p == "/apis/rbac.authorization.k8s.io/v1beta1/namespaces/federation-system/rolebindings" && m == http.MethodPost: + body, err := ioutil.ReadAll(req.Body) + if err != nil { + return nil, err + } + var got rbacv1beta1.RoleBinding + _, _, err = codec.Decode(body, nil, &got) + if err != nil { + return nil, err + } + if !api.Semantic.DeepEqual(got, rolebinding) { + return nil, fmt.Errorf("Unexpected rolebinding object\n\tDiff: %s", diff.ObjectGoPrintDiff(got, rolebinding)) + } + return &http.Response{StatusCode: http.StatusCreated, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(rbacCodec, &rolebinding)}, nil default: return nil, fmt.Errorf("unexpected request: %#v\n%#v", req.URL, req) } From 0dde763053b26558ec92d80ed89c9dae4dcf8523 Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Thu, 26 Jan 2017 20:44:15 -0800 Subject: [PATCH 5/7] Update bazel files. --- federation/pkg/kubefed/init/BUILD | 2 ++ 1 file changed, 2 insertions(+) diff --git a/federation/pkg/kubefed/init/BUILD b/federation/pkg/kubefed/init/BUILD index cb11d1e950b..b9d45bed50d 100644 --- a/federation/pkg/kubefed/init/BUILD +++ b/federation/pkg/kubefed/init/BUILD @@ -18,6 +18,7 @@ go_library( "//pkg/api:go_default_library", "//pkg/api/resource:go_default_library", "//pkg/apis/extensions:go_default_library", + "//pkg/apis/rbac:go_default_library", "//pkg/client/clientset_generated/internalclientset:go_default_library", "//pkg/kubectl/cmd/templates:go_default_library", "//pkg/kubectl/cmd/util:go_default_library", @@ -46,6 +47,7 @@ go_test( "//pkg/api/testapi:go_default_library", "//pkg/api/v1:go_default_library", "//pkg/apis/extensions/v1beta1:go_default_library", + "//pkg/apis/rbac/v1beta1:go_default_library", "//pkg/kubectl/cmd/testing:go_default_library", "//pkg/kubectl/cmd/util:go_default_library", "//pkg/util/intstr:go_default_library", From 4aeef0c7be72164caa7449d34f790278c729d703 Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Fri, 27 Jan 2017 14:51:58 -0800 Subject: [PATCH 6/7] Comment fixes. --- federation/pkg/kubefed/init/init.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/federation/pkg/kubefed/init/init.go b/federation/pkg/kubefed/init/init.go index 30fe14a1769..274dd373af6 100644 --- a/federation/pkg/kubefed/init/init.go +++ b/federation/pkg/kubefed/init/init.go @@ -67,7 +67,8 @@ const ( // calls to federation API server. ControllerManagerUser = "federation-controller-manager" - // Name of the ServiceAccount used by the federation controller manager. + // Name of the ServiceAccount used by the federation controller manager + // to access the secrets in the host cluster. ControllerManagerSA = "federation-controller-manager" // Group name of the legacy/core API group @@ -231,7 +232,8 @@ func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Comman } // 7. Create federation controller manager - // 7a. Create service accounts for federation controller manager + // 7a. Create a service account in the host cluster for federation + // controller manager. sa, err := createControllerManagerSA(hostClientset, initFlags.FederationSystemNamespace, dryRun) if err != nil { return err From 05a0f6490346cd542993449fa031393c2a30cb70 Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Mon, 30 Jan 2017 14:31:15 -0800 Subject: [PATCH 7/7] Address review comments. --- federation/pkg/kubefed/init/init.go | 6 ++++-- pkg/apis/rbac/helpers.go | 11 +++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/federation/pkg/kubefed/init/init.go b/federation/pkg/kubefed/init/init.go index 274dd373af6..5ded78a736c 100644 --- a/federation/pkg/kubefed/init/init.go +++ b/federation/pkg/kubefed/init/init.go @@ -576,8 +576,10 @@ func createRoleBindings(clientset *client.Clientset, namespace, saName string, d }, } - rolebinding := rbac.NewRoleBinding(roleName, namespace).SAs(namespace, saName).BindingOrDie() - rolebinding.Namespace = namespace + rolebinding, err := rbac.NewRoleBinding(roleName, namespace).SAs(namespace, saName).Binding() + if err != nil { + return nil, nil, err + } rolebinding.Labels = componentLabel if dryRun { diff --git a/pkg/apis/rbac/helpers.go b/pkg/apis/rbac/helpers.go index e04c22cbbf9..6beec911f5b 100644 --- a/pkg/apis/rbac/helpers.go +++ b/pkg/apis/rbac/helpers.go @@ -262,6 +262,10 @@ type RoleBindingBuilder struct { RoleBinding RoleBinding } +// NewRoleBinding creates a RoleBinding builder that can be used +// to define the subjects of a role binding. At least one of +// the `Groups`, `Users` or `SAs` method must be called before +// calling the `Binding*` methods. func NewRoleBinding(roleName, namespace string) *RoleBindingBuilder { return &RoleBindingBuilder{ RoleBinding: RoleBinding{ @@ -278,6 +282,7 @@ func NewRoleBinding(roleName, namespace string) *RoleBindingBuilder { } } +// Groups adds the specified groups as the subjects of the RoleBinding. func (r *RoleBindingBuilder) Groups(groups ...string) *RoleBindingBuilder { for _, group := range groups { r.RoleBinding.Subjects = append(r.RoleBinding.Subjects, Subject{Kind: GroupKind, Name: group}) @@ -285,6 +290,7 @@ func (r *RoleBindingBuilder) Groups(groups ...string) *RoleBindingBuilder { return r } +// Users adds the specified users as the subjects of the RoleBinding. func (r *RoleBindingBuilder) Users(users ...string) *RoleBindingBuilder { for _, user := range users { r.RoleBinding.Subjects = append(r.RoleBinding.Subjects, Subject{Kind: UserKind, Name: user}) @@ -292,6 +298,8 @@ func (r *RoleBindingBuilder) Users(users ...string) *RoleBindingBuilder { return r } +// SAs adds the specified service accounts as the subjects of the +// RoleBinding. func (r *RoleBindingBuilder) SAs(namespace string, serviceAccountNames ...string) *RoleBindingBuilder { for _, saName := range serviceAccountNames { r.RoleBinding.Subjects = append(r.RoleBinding.Subjects, Subject{Kind: ServiceAccountKind, Namespace: namespace, Name: saName}) @@ -299,6 +307,7 @@ func (r *RoleBindingBuilder) SAs(namespace string, serviceAccountNames ...string return r } +// BindingOrDie calls the binding method and panics if there is an error. func (r *RoleBindingBuilder) BindingOrDie() RoleBinding { ret, err := r.Binding() if err != nil { @@ -307,6 +316,8 @@ func (r *RoleBindingBuilder) BindingOrDie() RoleBinding { return ret } +// Binding builds and returns the RoleBinding API object from the builder +// object. func (r *RoleBindingBuilder) Binding() (RoleBinding, error) { if len(r.RoleBinding.Subjects) == 0 { return RoleBinding{}, fmt.Errorf("subjects are required: %#v", r.RoleBinding)