Skip to content

Commit

Permalink
Make it possible to specify custom service account
Browse files Browse the repository at this point in the history
What this PR does / why we need it:
This PR adds a new switch to the kudo cli --serviceaccount, which enables users to pass an existing service account instead of using default "Kudo-manager" service account.

Fixes #902
  • Loading branch information
sivaramsk authored and alenkacz committed Dec 6, 2019
1 parent 5e38fb3 commit 0566427
Show file tree
Hide file tree
Showing 7 changed files with 598 additions and 28 deletions.
32 changes: 18 additions & 14 deletions pkg/kudoctl/cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,23 +48,26 @@ and finishes with success if KUDO is already installed.
kubectl kudo init --crd-only
# delete crds
kubectl kudo init --crd-only --dry-run --output yaml | kubectl delete -f -
# pass existing serviceaccount
kubectl kudo init --service-account testaccount
`
)

type initCmd struct {
out io.Writer
fs afero.Fs
image string
dryRun bool
output string
version string
ns string
wait bool
timeout int64
clientOnly bool
crdOnly bool
home kudohome.Home
client *kube.Client
out io.Writer
fs afero.Fs
image string
dryRun bool
output string
version string
ns string
serviceAccount string
wait bool
timeout int64
clientOnly bool
crdOnly bool
home kudohome.Home
client *kube.Client
}

func newInitCmd(fs afero.Fs, out io.Writer) *cobra.Command {
Expand Down Expand Up @@ -98,6 +101,7 @@ func newInitCmd(fs afero.Fs, out io.Writer) *cobra.Command {
f.BoolVar(&i.crdOnly, "crd-only", false, "Add only KUDO CRDs to your cluster")
f.BoolVarP(&i.wait, "wait", "w", false, "Block until KUDO manager is running and ready to receive requests")
f.Int64Var(&i.timeout, "wait-timeout", 300, "Wait timeout to be used")
f.StringVarP(&i.serviceAccount, "service-account", "", "", "Override for the default serviceAccount kudo-manager")

return cmd
}
Expand All @@ -124,7 +128,7 @@ func (initCmd *initCmd) validate(flags *flag.FlagSet) error {

// run initializes local config and installs KUDO manager to Kubernetes cluster.
func (initCmd *initCmd) run() error {
opts := cmdInit.NewOptions(initCmd.version, initCmd.ns)
opts := cmdInit.NewOptions(initCmd.version, initCmd.ns, initCmd.serviceAccount)
// if image provided switch to it.
if initCmd.image != "" {
opts.Image = initCmd.image
Expand Down
20 changes: 14 additions & 6 deletions pkg/kudoctl/cmd/init/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ import (
//Defines the deployment of the KUDO manager and it's service definition.

const (
group = "kudo.dev"
crdVersion = "v1beta1"
defaultns = "kudo-system"
defaultGracePeriod = 10
group = "kudo.dev"
crdVersion = "v1beta1"
defaultns = "kudo-system"
defaultGracePeriod = 10
defaultServiceAccount = "kudo-manager"
)

// Options is the configurable options to init
Expand All @@ -40,10 +41,12 @@ type Options struct {
TerminationGracePeriodSeconds int64
// Image defines the image to be used
Image string
// ServiceAccount defines the optional serviceaccount to be used to install KUDO
ServiceAccount string
}

// NewOptions provides an option struct with defaults
func NewOptions(v string, ns string) Options {
func NewOptions(v string, ns string, sa string) Options {

if v == "" {
v = version.Get().GitVersion
Expand All @@ -52,11 +55,16 @@ func NewOptions(v string, ns string) Options {
ns = defaultns
}

if sa == "" {
sa = defaultServiceAccount
}

return Options{
Version: v,
Namespace: ns,
TerminationGracePeriodSeconds: defaultGracePeriod,
Image: fmt.Sprintf("kudobuilder/controller:v%v", v),
ServiceAccount: sa,
}
}

Expand Down Expand Up @@ -175,7 +183,7 @@ func generateDeployment(opts Options) *appsv1.StatefulSet {
Labels: labels,
},
Spec: v1.PodSpec{
ServiceAccountName: "kudo-manager",
ServiceAccountName: opts.ServiceAccount,
Containers: []v1.Container{

{
Expand Down
63 changes: 56 additions & 7 deletions pkg/kudoctl/cmd/init/prereqs.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,29 @@ import (

// Install uses Kubernetes client to install KUDO manager prereqs.
func installPrereqs(client kubernetes.Interface, opts Options) error {

if err := installNamespace(client.CoreV1(), opts); err != nil {
return err
}

if err := installServiceAccount(client.CoreV1(), opts); err != nil {
return err
}
if err := installRoleBindings(client, opts); err != nil {
return err
if opts.ServiceAccount != "kudo-manager" {
// Validate alternate serviceaccount exists in the cluster
if err := validateServiceAccountExists(client.CoreV1(), opts); err != nil {
return err
}
// Validate the alternate serviceaccount has cluster-admin clusterrolebinding
if err := validateClusterAdminRoleForSA(client, opts); err != nil {
return err
}
} else {
if err := installServiceAccount(client.CoreV1(), opts); err != nil {
return err
}
if err := installRoleBindings(client, opts); err != nil {
return err
}
}

if err := installSecret(client.CoreV1(), opts); err != nil {
return err
}
Expand Down Expand Up @@ -106,7 +119,7 @@ func generateServiceAccount(opts Options) *v1.ServiceAccount {
sa := &v1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Labels: labels,
Name: "kudo-manager",
Name: opts.ServiceAccount,
Namespace: opts.Namespace,
},
}
Expand All @@ -127,7 +140,7 @@ func generateRoleBinding(opts Options) *rbacv1.ClusterRoleBinding {
},
Subjects: []rbacv1.Subject{{
Kind: "ServiceAccount",
Name: "kudo-manager",
Name: opts.ServiceAccount,
Namespace: opts.Namespace,
}},
}
Expand Down Expand Up @@ -223,3 +236,39 @@ func namespace(namespace string) *v1.Namespace {
}
return ns
}

// Validate whether the serviceAccount exists
func validateServiceAccountExists(client corev1.ServiceAccountsGetter, opts Options) error {

saList, err := client.ServiceAccounts(opts.Namespace).List(metav1.ListOptions{})
if err != nil {
return err
}
for _, sa := range saList.Items {
if sa.Name == opts.ServiceAccount {
return nil
}
}

return fmt.Errorf("Service Account %s does not exists - KUDO expects the serviceAccount to be present in the namespace %s", opts.ServiceAccount, opts.Namespace)
}

// Validate whether the serviceAccount has cluster-admin role
func validateClusterAdminRoleForSA(client kubernetes.Interface, opts Options) error {

// Check whether the serviceAccount has clusterrolebinding cluster-admin
crbs, err := client.RbacV1().ClusterRoleBindings().List(metav1.ListOptions{})
if err != nil {
return err
}

for _, crb := range crbs.Items {
for _, subject := range crb.Subjects {
if subject.Name == opts.ServiceAccount && subject.Namespace == opts.Namespace && crb.RoleRef.Name == "cluster-admin" {
return nil
}
}
}

return fmt.Errorf("Service Account %s does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace %s and to have cluster-admin role", opts.ServiceAccount, opts.Namespace)
}
117 changes: 116 additions & 1 deletion pkg/kudoctl/cmd/init_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,121 @@ func TestIntegInitWithNameSpace(t *testing.T) {
assert.True(t, kudoControllerFound, fmt.Sprintf("No kudo-controller-manager statefulset found in namespace %s", namespace))
}

/*
Test the below 5 scenarios
1. Run Init command with a serviceAccount that is not present in the cluster.
2. Run init command with a serviceAccount that is present in the cluster, but not in the clusterrole-binding.
3. Run Init command with a serviceAccount that does not have cluster-admin role.
4. Run Init command with a serviceAccount that does have cluster-admin role, but not in the expected namespace.
5. Run Init command with a serviceAccount that is present in the cluster and also has cluster-admin role.
*/

func TestIntegInitWithServiceAccount(t *testing.T) {
namespace := "sa-integration-test"
serviceAccount := "sa-integration"
// Kubernetes client caches the types, se we need to re-initialize it.
testClient, err := testutils.NewRetryClient(testenv.Config, client.Options{
Scheme: testutils.Scheme(),
})
assert.Nil(t, err)
kclient := getKubeClient(t)

instance := testutils.NewResource("kudo.dev/v1beta1", "Instance", "zk", "ns")
// Verify that we cannot create the instance, because the test environment is empty.
assert.IsType(t, &meta.NoKindMatchError{}, testClient.Create(context.TODO(), instance))

// Install all of the CRDs.
crds := cmdinit.CRDs().AsArray()
defer deleteInitObjects(testClient)

var buf bytes.Buffer
cmd := &initCmd{
out: &buf,
fs: afero.NewMemMapFs(),
client: kclient,
ns: namespace,
serviceAccount: "test-account",
}

// Manually create the namespace and the serviceAccount to be used later
ns := testutils.NewResource("v1", "Namespace", namespace, "")
assert.NoError(t, testClient.Create(context.TODO(), ns))
defer testClient.Delete(context.TODO(), ns)
sa := testutils.NewResource("v1", "ServiceAccount", serviceAccount, namespace)
assert.NoError(t, testClient.Create(context.TODO(), sa))
defer testClient.Delete(context.TODO(), sa)

// Test Case 1, the serviceAccount does not exist, expect serviceAccount not exists error
err = cmd.run()
require.Error(t, err)
assert.Equal(t, err.Error(), `error installing: Service Account test-account does not exists - KUDO expects the serviceAccount to be present in the namespace sa-integration-test`)

// Create the serviceAccount, in the default namespace.
ns2 := testutils.NewResource("v1", "Namespace", "test-ns", "")
assert.NoError(t, testClient.Create(context.TODO(), ns2))
defer testClient.Delete(context.TODO(), ns2)
sa2 := testutils.NewResource("v1", "ServiceAccount", "sa-nonadmin", "test-ns")
assert.NoError(t, testClient.Create(context.TODO(), sa2))
defer testClient.Delete(context.TODO(), sa2)

// Test Case 2, the serviceAccount exists, but does not part of clusterrolebindings
cmd.serviceAccount = "sa-nonadmin"
cmd.ns = "test-ns"
err = cmd.run()
require.Error(t, err)
assert.Equal(t, err.Error(), `error installing: Service Account sa-nonadmin does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace test-ns and to have cluster-admin role`)

// Test case 3: Run Init command with a serviceAccount that does not have cluster-admin role.
cmd.serviceAccount = serviceAccount
cmd.ns = namespace
crb := testutils.NewClusterRoleBinding("rbac.authorization.k8s.io/v1", "ClusterRoleBinding", "kudo-test1", "test-ns", serviceAccount, "cluster-temp")
assert.NoError(t, testClient.Create(context.TODO(), crb))
defer testClient.Delete(context.TODO(), crb)

err = cmd.run()
require.Error(t, err)
assert.Equal(t, err.Error(), `error installing: Service Account sa-integration does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace sa-integration-test and to have cluster-admin role`)

// Test case 4: Run Init command with a serviceAccount that does not have cluster-admin role.
crb2 := testutils.NewClusterRoleBinding("rbac.authorization.k8s.io/v1", "ClusterRoleBinding", "kudo-test2", namespace, serviceAccount, "cluster-temp")
assert.NoError(t, testClient.Create(context.TODO(), crb2))
defer testClient.Delete(context.TODO(), crb2)

err = cmd.run()
require.Error(t, err)
assert.Equal(t, err.Error(), `error installing: Service Account sa-integration does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace sa-integration-test and to have cluster-admin role`)

// Test case 5: Run Init command with a serviceAccount that is present in the cluster and also has cluster-admin role.
crb3 := testutils.NewClusterRoleBinding("rbac.authorization.k8s.io/v1", "ClusterRoleBinding", "kudo-clusterrole-binding", namespace, serviceAccount, "cluster-admin")
assert.NoError(t, testClient.Create(context.TODO(), crb3))
defer testClient.Delete(context.TODO(), crb3)

err = cmd.run()
assert.NoError(t, err)

// WaitForCRDs to be created... the init cmd did NOT wait
assert.Nil(t, testutils.WaitForCRDs(testenv.DiscoveryClient, crds))

// Kubernetes client caches the types, so we need to re-initialize it.
testClient, err = testutils.NewRetryClient(testenv.Config, client.Options{
Scheme: testutils.Scheme(),
})
assert.Nil(t, err)
kclient = getKubeClient(t)

// make sure that the controller lives in the correct namespace
statefulsets, err := kclient.KubeClient.AppsV1().StatefulSets(namespace).List(metav1.ListOptions{})
assert.Nil(t, err)

kudoControllerFound := false
for _, ss := range statefulsets.Items {
if ss.Name == "kudo-controller-manager" {
kudoControllerFound = true
}
}
assert.True(t, kudoControllerFound, fmt.Sprintf("No kudo-controller-manager statefulset found in namespace %s", namespace))
}

func TestNoErrorOnReInit(t *testing.T) {
// if the CRD exists and we init again there should be no error
testClient, err := testutils.NewRetryClient(testenv.Config, client.Options{
Expand Down Expand Up @@ -256,7 +371,7 @@ func TestNoErrorOnReInit(t *testing.T) {

func deleteInitObjects(client *testutils.RetryClient) {
crds := cmdinit.CRDs().AsArray()
prereqs := cmdinit.Prereq(cmdinit.NewOptions("", ""))
prereqs := cmdinit.Prereq(cmdinit.NewOptions("", "", ""))
deleteCRDs(crds, client)
deletePrereq(prereqs, client)
}
Expand Down
36 changes: 36 additions & 0 deletions pkg/kudoctl/cmd/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,42 @@ func TestInitCmd_CustomNamespace(t *testing.T) {
}
}

func TestInitCmd_ServiceAccount(t *testing.T) {
file := "deploy-kudo-sa.yaml"
fs := afero.NewMemMapFs()
out := &bytes.Buffer{}
initCmd := newInitCmd(fs, out)
Settings.AddFlags(initCmd.Flags())
flags := map[string]string{"dry-run": "true", "output": "yaml", "service-account": "safoo", "namespace": "foo"}

for flag, value := range flags {
if err := initCmd.Flags().Set(flag, value); err != nil {
t.Fatal(err)
}
}
if err := initCmd.RunE(initCmd, []string{}); err != nil {
t.Fatal(err)
}

gp := filepath.Join("testdata", file+".golden")

if *updateGolden {
t.Log("update golden file")
if err := ioutil.WriteFile(gp, out.Bytes(), 0644); err != nil {
t.Fatalf("failed to update golden file: %s", err)
}
}
g, err := ioutil.ReadFile(gp)
if err != nil {
t.Fatalf("failed reading .golden: %s", err)
}

if !bytes.Equal(out.Bytes(), g) {
assert.Equal(t, string(g), out.String(), "for golden file: %s", gp)
}

}

func TestNewInitCmd(t *testing.T) {
fs := afero.NewMemMapFs()
var tests = []struct {
Expand Down
Loading

0 comments on commit 0566427

Please sign in to comment.