From 69dad38f66b00def9269cfa00fafe5c86275ac65 Mon Sep 17 00:00:00 2001 From: Andreas Neumann Date: Mon, 4 May 2020 12:01:04 +0200 Subject: [PATCH 1/4] Extracted kudoinit refactoring Signed-off-by: Andreas Neumann --- .../instance/instance_controller_test.go | 30 +-- pkg/kudoctl/cmd/init.go | 23 ++ pkg/kudoctl/cmd/init_integration_test.go | 27 ++- pkg/kudoctl/kudoinit/crd/crds.go | 35 ++-- pkg/kudoctl/kudoinit/manager/manager.go | 5 +- pkg/kudoctl/kudoinit/prereq/namespace.go | 42 ++-- pkg/kudoctl/kudoinit/prereq/namespace_test.go | 30 ++- pkg/kudoctl/kudoinit/prereq/prereqs.go | 76 ------- pkg/kudoctl/kudoinit/prereq/prereqs_test.go | 68 ------ pkg/kudoctl/kudoinit/prereq/serviceaccount.go | 89 ++++---- .../kudoinit/prereq/serviceaccount_test.go | 61 +++++- pkg/kudoctl/kudoinit/prereq/webhook.go | 198 +++++++++--------- pkg/kudoctl/kudoinit/prereq/webhook_test.go | 30 +-- pkg/kudoctl/kudoinit/setup/setup.go | 25 +-- pkg/kudoctl/kudoinit/types.go | 5 +- pkg/kudoctl/util/kudo/kudo.go | 15 +- 16 files changed, 363 insertions(+), 396 deletions(-) delete mode 100644 pkg/kudoctl/kudoinit/prereq/prereqs.go diff --git a/pkg/controller/instance/instance_controller_test.go b/pkg/controller/instance/instance_controller_test.go index ff6cd5223..128b89e73 100644 --- a/pkg/controller/instance/instance_controller_test.go +++ b/pkg/controller/instance/instance_controller_test.go @@ -175,9 +175,9 @@ func Test_makePipes(t *testing.T) { plan: &v1beta1.Plan{Strategy: "serial", Phases: []v1beta1.Phase{ { Name: "phase", Strategy: "serial", Steps: []v1beta1.Step{ - { - Name: "step", Tasks: []string{}}, - }}, + { + Name: "step", Tasks: []string{}}, + }}, }}, tasks: []v1beta1.Task{}, emeta: meta, @@ -189,9 +189,9 @@ func Test_makePipes(t *testing.T) { plan: &v1beta1.Plan{Strategy: "serial", Phases: []v1beta1.Phase{ { Name: "phase", Strategy: "serial", Steps: []v1beta1.Step{ - { - Name: "step", Tasks: []string{"task"}}, - }}, + { + Name: "step", Tasks: []string{"task"}}, + }}, }}, tasks: []v1beta1.Task{ { @@ -211,9 +211,9 @@ func Test_makePipes(t *testing.T) { plan: &v1beta1.Plan{Strategy: "serial", Phases: []v1beta1.Phase{ { Name: "phase", Strategy: "serial", Steps: []v1beta1.Step{ - { - Name: "step", Tasks: []string{"task"}}, - }}, + { + Name: "step", Tasks: []string{"task"}}, + }}, }}, tasks: []v1beta1.Task{ { @@ -242,9 +242,9 @@ func Test_makePipes(t *testing.T) { plan: &v1beta1.Plan{Strategy: "serial", Phases: []v1beta1.Phase{ { Name: "phase", Strategy: "serial", Steps: []v1beta1.Step{ - {Name: "stepOne", Tasks: []string{"task-one"}}, - {Name: "stepTwo", Tasks: []string{"task-two"}}, - }}, + {Name: "stepOne", Tasks: []string{"task-one"}}, + {Name: "stepTwo", Tasks: []string{"task-two"}}, + }}, }}, tasks: []v1beta1.Task{ { @@ -292,9 +292,9 @@ func Test_makePipes(t *testing.T) { plan: &v1beta1.Plan{Strategy: "serial", Phases: []v1beta1.Phase{ { Name: "phase", Strategy: "serial", Steps: []v1beta1.Step{ - { - Name: "step", Tasks: []string{"task"}}, - }}, + { + Name: "step", Tasks: []string{"task"}}, + }}, }}, tasks: []v1beta1.Task{ { diff --git a/pkg/kudoctl/cmd/init.go b/pkg/kudoctl/cmd/init.go index 3cdac2ee7..a120da158 100644 --- a/pkg/kudoctl/cmd/init.go +++ b/pkg/kudoctl/cmd/init.go @@ -6,6 +6,8 @@ import ( "io" "strings" + "github.com/kudobuilder/kudo/pkg/kudoctl/verifier" + "github.com/spf13/afero" "github.com/spf13/cobra" flag "github.com/spf13/pflag" @@ -179,6 +181,13 @@ func (initCmd *initCmd) run() error { } initCmd.client = client } + ok, err := initCmd.preInstallVerify(opts) + if err != nil { + return err + } + if !ok { + return fmt.Errorf("failed to verify installation requirements") + } if err := setup.Install(initCmd.client, opts, initCmd.crdOnly); err != nil { return clog.Errorf("error installing: %s", err) @@ -196,6 +205,20 @@ func (initCmd *initCmd) run() error { return nil } +// preInstallVerify runs the pre-installation verification and returns true if the installation can continue +func (initCmd *initCmd) preInstallVerify(opts kudoinit.Options) (bool, error) { + result := verifier.NewResult() + if err := setup.PreInstallVerify(initCmd.client, opts, initCmd.crdOnly, &result); err != nil { + return false, err + } + result.PrintWarnings(initCmd.out) + if !result.IsValid() { + result.PrintErrors(initCmd.out) + return false, nil + } + return true, nil +} + func webhooksArray(webhooksAsStr string) []string { if webhooksAsStr == "" { return []string{} diff --git a/pkg/kudoctl/cmd/init_integration_test.go b/pkg/kudoctl/cmd/init_integration_test.go index 66dc47073..cb32dccbe 100644 --- a/pkg/kudoctl/cmd/init_integration_test.go +++ b/pkg/kudoctl/cmd/init_integration_test.go @@ -71,6 +71,10 @@ func assertManifestFileMatch(t *testing.T, fileName string, expectedObject runti assert.Equal(t, string(expectedContent), string(of), fmt.Sprintf("embedded file %s does not match the source, run 'make generate'", fileName)) } +func assertStringContains(t *testing.T, expected string, actual string) { + assert.True(t, strings.Contains(actual, expected), "Expected to find '%s' in '%s'", expected, actual) +} + func runtimeObjectAsBytes(o runtime.Object) ([]byte, error) { bytes, err := yaml.Marshal(o) if err != nil { @@ -145,7 +149,8 @@ func TestIntegInitWithNameSpace(t *testing.T) { // On first attempt, the namespace does not exist, so the error is expected. err = cmd.run() require.Error(t, err) - assert.Equal(t, "error installing: Namespace integration-test does not exist - KUDO expects that any namespace except the default kudo-system is created beforehand\n", err.Error()) + assert.Equal(t, "failed to verify installation requirements", err.Error()) + assertStringContains(t, "Namespace integration-test does not exist - KUDO expects that any namespace except the default kudo-system is created beforehand", buf.String()) // Then we manually create the namespace. ns := testutils.NewResource("v1", "Namespace", namespace, "") @@ -226,7 +231,8 @@ func TestIntegInitWithServiceAccount(t *testing.T) { // Test Case 1, the serviceAccount does not exist, expect serviceAccount not exists error err = cmd.run() require.Error(t, err) - assert.Equal(t, "error installing: Service Account test-account does not exists - KUDO expects the serviceAccount to be present in the namespace sa-integration-test\n", err.Error()) + assert.Equal(t, "failed to verify installation requirements", err.Error()) + assertStringContains(t, "Service Account test-account does not exists - KUDO expects the serviceAccount to be present in the namespace sa-integration-test", buf.String()) // Create the serviceAccount, in the default namespace. ns2 := testutils.NewResource("v1", "Namespace", "test-ns", "") @@ -241,7 +247,8 @@ func TestIntegInitWithServiceAccount(t *testing.T) { cmd.ns = "test-ns" err = cmd.run() require.Error(t, err) - assert.Equal(t, "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\n", err.Error()) + assert.Equal(t, "failed to verify installation requirements", err.Error()) + assertStringContains(t, "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", buf.String()) // Test case 3: Run Init command with a serviceAccount that does not have cluster-admin role. cmd.serviceAccount = serviceAccount @@ -252,7 +259,8 @@ func TestIntegInitWithServiceAccount(t *testing.T) { err = cmd.run() require.Error(t, err) - assert.Equal(t, "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\n", err.Error()) + assert.Equal(t, "failed to verify installation requirements", err.Error()) + assertStringContains(t, "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", buf.String()) // 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") @@ -261,7 +269,8 @@ func TestIntegInitWithServiceAccount(t *testing.T) { err = cmd.run() require.Error(t, err) - assert.Equal(t, "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\n", err.Error()) + assert.Equal(t, "failed to verify installation requirements", err.Error()) + assertStringContains(t, "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", buf.String()) // 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") @@ -340,10 +349,14 @@ func TestNoErrorOnReInit(t *testing.T) { } func deleteInitObjects(client *testutils.RetryClient) { + opts := kudoinit.NewOptions("", "", "", []string{}, false) + crds := crd.NewInitializer() - prereqs := prereq.NewInitializer(kudoinit.NewOptions("", "", "", []string{}, false)) + deleteCRDs(crds.Resources(), client) - deletePrereq(prereqs.Resources(), client) + deletePrereq(prereq.NewNamespaceInitializer(opts).Resources(), client) + deletePrereq(prereq.NewServiceAccountInitializer(opts).Resources(), client) + deletePrereq(prereq.NewWebHookInitializer(opts).Resources(), client) } func deleteCRDs(crds []runtime.Object, client *testutils.RetryClient) { diff --git a/pkg/kudoctl/kudoinit/crd/crds.go b/pkg/kudoctl/kudoinit/crd/crds.go index 4e65bde80..d86301344 100644 --- a/pkg/kudoctl/kudoinit/crd/crds.go +++ b/pkg/kudoctl/kudoinit/crd/crds.go @@ -7,7 +7,7 @@ import ( apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1" - kerrors "k8s.io/apimachinery/pkg/api/errors" + k8serrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/yaml" @@ -46,54 +46,59 @@ func (c Initializer) Resources() []runtime.Object { return []runtime.Object{c.Operator, c.OperatorVersion, c.Instance} } -func (c Initializer) PreInstallVerify(client *kube.Client) verifier.Result { - return verifier.NewResult() +func (c Initializer) PreInstallVerify(client *kube.Client, result *verifier.Result) error { + return nil } -// Install uses Kubernetes client to install KUDO Crds. -func (c Initializer) Install(client *kube.Client) error { - if err := c.install(client.ExtClient.ApiextensionsV1beta1(), c.Operator); err != nil { +func (c Initializer) VerifyInstallation(client *kube.Client, result *verifier.Result) error { + if err := c.verifyInstallation(client.ExtClient.ApiextensionsV1beta1(), c.Operator, result); err != nil { return err } - if err := c.install(client.ExtClient.ApiextensionsV1beta1(), c.OperatorVersion); err != nil { + if err := c.verifyInstallation(client.ExtClient.ApiextensionsV1beta1(), c.OperatorVersion, result); err != nil { return err } - if err := c.install(client.ExtClient.ApiextensionsV1beta1(), c.Instance); err != nil { + if err := c.verifyInstallation(client.ExtClient.ApiextensionsV1beta1(), c.Instance, result); err != nil { return err } return nil } -func (c Initializer) ValidateInstallation(client *kube.Client) error { - if err := c.validateInstallation(client.ExtClient.ApiextensionsV1beta1(), c.Operator); err != nil { +// Install uses Kubernetes client to install KUDO Crds. +func (c Initializer) Install(client *kube.Client) error { + if err := c.install(client.ExtClient.ApiextensionsV1beta1(), c.Operator); err != nil { return err } - if err := c.validateInstallation(client.ExtClient.ApiextensionsV1beta1(), c.OperatorVersion); err != nil { + if err := c.install(client.ExtClient.ApiextensionsV1beta1(), c.OperatorVersion); err != nil { return err } - if err := c.validateInstallation(client.ExtClient.ApiextensionsV1beta1(), c.Instance); err != nil { + if err := c.install(client.ExtClient.ApiextensionsV1beta1(), c.Instance); err != nil { return err } return nil } -func (c Initializer) validateInstallation(client v1beta1.CustomResourceDefinitionsGetter, crd *apiextv1beta1.CustomResourceDefinition) error { +func (c Initializer) verifyInstallation(client v1beta1.CustomResourceDefinitionsGetter, crd *apiextv1beta1.CustomResourceDefinition, result *verifier.Result) error { existingCrd, err := client.CustomResourceDefinitions().Get(crd.Name, v1.GetOptions{}) if err != nil { if os.IsTimeout(err) { return err } + if k8serrors.IsNotFound(err) { + result.AddErrors(fmt.Sprintf("CRD %s is not installed", crd.Name)) + return nil + } return fmt.Errorf("failed to retrieve CRD %s: %v", crd.Name, err) } if existingCrd.Spec.Version != crd.Spec.Version { - return fmt.Errorf("installed CRD %s has invalid version %s, expected %s", crd.Name, existingCrd.Spec.Version, crd.Spec.Version) + result.AddErrors(fmt.Sprintf("Installed CRD %s has invalid version %s, expected %s", crd.Name, existingCrd.Spec.Version, crd.Spec.Version)) + return nil } return nil } func (c Initializer) install(client v1beta1.CustomResourceDefinitionsGetter, crd *apiextv1beta1.CustomResourceDefinition) error { _, err := client.CustomResourceDefinitions().Create(crd) - if kerrors.IsAlreadyExists(err) { + if k8serrors.IsAlreadyExists(err) { clog.V(4).Printf("crd %v already exists", crd.Name) return nil } diff --git a/pkg/kudoctl/kudoinit/manager/manager.go b/pkg/kudoctl/kudoinit/manager/manager.go index 7b7ef6431..683b269e1 100644 --- a/pkg/kudoctl/kudoinit/manager/manager.go +++ b/pkg/kudoctl/kudoinit/manager/manager.go @@ -40,8 +40,9 @@ func NewInitializer(options kudoinit.Options) Initializer { } } -func (m Initializer) PreInstallVerify(client *kube.Client) verifier.Result { - return verifier.NewResult() +func (m Initializer) PreInstallVerify(client *kube.Client, result *verifier.Result) error { + // Nothing to verify yet + return nil } func (m Initializer) String() string { diff --git a/pkg/kudoctl/kudoinit/prereq/namespace.go b/pkg/kudoctl/kudoinit/prereq/namespace.go index b67f7ca1b..717e57d2d 100644 --- a/pkg/kudoctl/kudoinit/prereq/namespace.go +++ b/pkg/kudoctl/kudoinit/prereq/namespace.go @@ -15,32 +15,40 @@ import ( ) // Ensure IF is implemented -var _ k8sResource = &kudoNamespace{} +var _ kudoinit.Step = &KudoNamespace{} -type kudoNamespace struct { +type KudoNamespace struct { opts kudoinit.Options ns *v1.Namespace } -func (o kudoNamespace) PreInstallVerify(client *kube.Client) verifier.Result { +func NewNamespaceInitializer(options kudoinit.Options) KudoNamespace { + return KudoNamespace{ + opts: options, + ns: generateSysNamespace(options.Namespace), + } +} + +func (o KudoNamespace) String() string { + return "namespace" +} + +func (o KudoNamespace) PreInstallVerify(client *kube.Client, result *verifier.Result) error { // We only manage kudo-system namespace. For others we expect they exist. if !o.opts.IsDefaultNamespace() { _, err := client.KubeClient.CoreV1().Namespaces().Get(o.opts.Namespace, metav1.GetOptions{}) - if kerrors.IsNotFound(err) { - return verifier.NewError(fmt.Sprintf("Namespace %s does not exist - KUDO expects that any namespace except the default %s is created beforehand", o.opts.Namespace, kudoinit.DefaultNamespace)) + if err != nil { + if kerrors.IsNotFound(err) { + result.AddErrors(fmt.Sprintf("Namespace %s does not exist - KUDO expects that any namespace except the default %s is created beforehand", o.opts.Namespace, kudoinit.DefaultNamespace)) + return nil + } + return err } } - return verifier.NewResult() -} - -func newNamespace(options kudoinit.Options) kudoNamespace { - return kudoNamespace{ - opts: options, - ns: generateSysNamespace(options.Namespace), - } + return nil } -func (o kudoNamespace) Install(client *kube.Client) error { +func (o KudoNamespace) Install(client *kube.Client) error { _, err := client.KubeClient.CoreV1().Namespaces().Create(o.ns) if kerrors.IsAlreadyExists(err) { clog.V(4).Printf("namespace %v already exists", o.ns.Name) @@ -49,11 +57,7 @@ func (o kudoNamespace) Install(client *kube.Client) error { return err } -func (o kudoNamespace) ValidateInstallation(client *kube.Client) error { - return nil -} - -func (o kudoNamespace) AsRuntimeObjs() []runtime.Object { +func (o KudoNamespace) Resources() []runtime.Object { if !o.opts.IsDefaultNamespace() { return make([]runtime.Object, 0) } diff --git a/pkg/kudoctl/kudoinit/prereq/namespace_test.go b/pkg/kudoctl/kudoinit/prereq/namespace_test.go index f5e5ff003..fd9c1afd2 100644 --- a/pkg/kudoctl/kudoinit/prereq/namespace_test.go +++ b/pkg/kudoctl/kudoinit/prereq/namespace_test.go @@ -3,6 +3,14 @@ package prereq import ( "testing" + core "k8s.io/api/core/v1" + v12 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + testing2 "k8s.io/client-go/testing" + + "github.com/kudobuilder/kudo/pkg/kudoctl/kube" + "github.com/stretchr/testify/assert" "github.com/kudobuilder/kudo/pkg/kudoctl/kudoinit" @@ -12,9 +20,9 @@ import ( func TestPrereq_Fail_PreValidate_CustomNamespace(t *testing.T) { client := getFakeClient() - init := NewInitializer(kudoinit.NewOptions("", "customNS", "", make([]string, 0), false)) - - result := init.PreInstallVerify(client) + init := NewNamespaceInitializer(kudoinit.NewOptions("", "customNS", "", make([]string, 0), false)) + result := verifier.NewResult() + _ = init.PreInstallVerify(client, &result) assert.EqualValues(t, verifier.NewError("Namespace customNS does not exist - KUDO expects that any namespace except the default kudo-system is created beforehand"), result) } @@ -24,9 +32,21 @@ func TestPrereq_Ok_PreValidate_CustomNamespace(t *testing.T) { mockGetNamespace(client, "customNS") - init := NewInitializer(kudoinit.NewOptions("", "customNS", "", make([]string, 0), false)) + init := NewNamespaceInitializer(kudoinit.NewOptions("", "customNS", "", make([]string, 0), false)) - result := init.PreInstallVerify(client) + result := verifier.NewResult() + _ = init.PreInstallVerify(client, &result) assert.EqualValues(t, verifier.NewResult(), result) } + +func mockGetNamespace(client *kube.Client, nsName string) { + client.KubeClient.(*fake.Clientset).Fake.PrependReactor("get", "namespaces", func(action testing2.Action) (handled bool, ret runtime.Object, err error) { + ns := &core.Namespace{ + ObjectMeta: v12.ObjectMeta{ + Name: nsName, + }, + } + return true, ns, nil + }) +} diff --git a/pkg/kudoctl/kudoinit/prereq/prereqs.go b/pkg/kudoctl/kudoinit/prereq/prereqs.go deleted file mode 100644 index ce14d7478..000000000 --- a/pkg/kudoctl/kudoinit/prereq/prereqs.go +++ /dev/null @@ -1,76 +0,0 @@ -package prereq - -import ( - "k8s.io/apimachinery/pkg/runtime" - - "github.com/kudobuilder/kudo/pkg/kudoctl/kube" - "github.com/kudobuilder/kudo/pkg/kudoctl/kudoinit" - "github.com/kudobuilder/kudo/pkg/kudoctl/verifier" -) - -// Ensure kudoinit.Step is implemented -var _ kudoinit.Step = &Initializer{} - -// Defines a single prerequisite that is defined as a k8s resource -type k8sResource interface { - // PreInstallVerify is called before the installation of any component is started and should return an error if the installation is not possible - PreInstallVerify(client *kube.Client) verifier.Result - - // Install installs the manifests of this prerequisite - Install(client *kube.Client) error - - // ValidateInstallation verifies that the current state of the installation is as expected of this version of KUDO - ValidateInstallation(client *kube.Client) error - - // AsRuntimeObjs returns the manifests that would be installed from this resource - AsRuntimeObjs() []runtime.Object -} - -//Defines the Prerequisites that need to be in place to run the KUDO manager. This includes setting up the kudo-system namespace and service account -type Initializer struct { - Options kudoinit.Options - prereqs []k8sResource -} - -func NewInitializer(options kudoinit.Options) Initializer { - return Initializer{ - Options: options, - prereqs: []k8sResource{ - newNamespace(options), - newServiceAccount(options), - newWebHook(options), - }, - } -} - -func (p Initializer) String() string { - return "service accounts and other requirements for controller to run" -} - -func (p Initializer) PreInstallVerify(client *kube.Client) verifier.Result { - result := verifier.NewResult() - for _, prereq := range p.prereqs { - res := prereq.PreInstallVerify(client) - result.Merge(res) - } - return result -} - -func (p Initializer) Install(client *kube.Client) error { - for _, prereq := range p.prereqs { - err := prereq.Install(client) - if err != nil { - return err - } - } - return nil -} - -func (p Initializer) Resources() []runtime.Object { - var prereqs []runtime.Object - - for _, prereq := range p.prereqs { - prereqs = append(prereqs, prereq.AsRuntimeObjs()...) - } - return prereqs -} diff --git a/pkg/kudoctl/kudoinit/prereq/prereqs_test.go b/pkg/kudoctl/kudoinit/prereq/prereqs_test.go index 820942d90..8abfe376e 100644 --- a/pkg/kudoctl/kudoinit/prereq/prereqs_test.go +++ b/pkg/kudoctl/kudoinit/prereq/prereqs_test.go @@ -1,21 +1,12 @@ package prereq import ( - "testing" - - "github.com/stretchr/testify/assert" - core "k8s.io/api/core/v1" - rbac "k8s.io/api/rbac/v1" apiextensionfake "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake" - v12 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" dynamicfake "k8s.io/client-go/dynamic/fake" "k8s.io/client-go/kubernetes/fake" - testing2 "k8s.io/client-go/testing" "github.com/kudobuilder/kudo/pkg/kudoctl/kube" - "github.com/kudobuilder/kudo/pkg/kudoctl/kudoinit" - "github.com/kudobuilder/kudo/pkg/kudoctl/verifier" ) func getFakeClient() *kube.Client { @@ -30,62 +21,3 @@ func getFakeClient() *kube.Client { ExtClient: extClient, DynamicClient: dynamicClient} } - -func TestPrereq_Ok_PreValidate_DefaultOpts(t *testing.T) { - client := getFakeClient() - - init := NewInitializer(kudoinit.NewOptions("", "", "", make([]string, 0), false)) - - result := init.PreInstallVerify(client) - - assert.EqualValues(t, verifier.NewResult(), result) -} - -func mockGetNamespace(client *kube.Client, nsName string) { - client.KubeClient.(*fake.Clientset).Fake.PrependReactor("get", "namespaces", func(action testing2.Action) (handled bool, ret runtime.Object, err error) { - ns := &core.Namespace{ - ObjectMeta: v12.ObjectMeta{ - Name: nsName, - }, - } - return true, ns, nil - }) -} - -func mockListServiceAccounts(client *kube.Client, saName string) { - client.KubeClient.(*fake.Clientset).Fake.PrependReactor("list", "serviceaccounts", func(action testing2.Action) (handled bool, ret runtime.Object, err error) { - sa := core.ServiceAccount{ - ObjectMeta: v12.ObjectMeta{ - Name: saName, - }, - Secrets: nil, - ImagePullSecrets: nil, - AutomountServiceAccountToken: nil, - } - sal := &core.ServiceAccountList{ - Items: []core.ServiceAccount{sa}, - } - return true, sal, nil - }) -} - -func mockListClusterRoleBindings(client *kube.Client, opts kudoinit.Options) { - client.KubeClient.(*fake.Clientset).Fake.PrependReactor("list", "clusterrolebindings", func(action testing2.Action) (handled bool, ret runtime.Object, err error) { - subject := rbac.Subject{ - Kind: "", - Name: opts.ServiceAccount, - Namespace: opts.Namespace, - } - - crb := rbac.ClusterRoleBinding{ - Subjects: []rbac.Subject{subject}, - RoleRef: rbac.RoleRef{ - Name: "cluster-admin", - }, - } - crbList := &rbac.ClusterRoleBindingList{ - Items: []rbac.ClusterRoleBinding{crb}, - } - return true, crbList, nil - }) -} diff --git a/pkg/kudoctl/kudoinit/prereq/serviceaccount.go b/pkg/kudoctl/kudoinit/prereq/serviceaccount.go index 5f110ebae..f0fefe22d 100644 --- a/pkg/kudoctl/kudoinit/prereq/serviceaccount.go +++ b/pkg/kudoctl/kudoinit/prereq/serviceaccount.go @@ -2,7 +2,6 @@ package prereq import ( "fmt" - "reflect" v1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -17,38 +16,45 @@ import ( ) // Ensure IF is implemented -var _ k8sResource = &kudoServiceAccount{} +var _ kudoinit.Step = &KudoServiceAccount{} -type kudoServiceAccount struct { +type KudoServiceAccount struct { opts kudoinit.Options serviceAccount *v1.ServiceAccount roleBinding *rbacv1.ClusterRoleBinding } -func newServiceAccount(options kudoinit.Options) kudoServiceAccount { - return kudoServiceAccount{ +func NewServiceAccountInitializer(options kudoinit.Options) KudoServiceAccount { + return KudoServiceAccount{ opts: options, serviceAccount: generateServiceAccount(options), roleBinding: generateRoleBinding(options), } } -func (o kudoServiceAccount) PreInstallVerify(client *kube.Client) verifier.Result { - result := verifier.NewResult() +func (o KudoServiceAccount) String() string { + return "service account" +} + +func (o KudoServiceAccount) PreInstallVerify(client *kube.Client, result *verifier.Result) error { if o.opts.IsDefaultServiceAccount() { - return result + return nil + } + if err := o.validateServiceAccountExists(client, result); err != nil { + return err } - result.Merge(o.validateServiceAccountExists(client)) if result.IsValid() { // Only validate role if SA is ok - result.Merge(o.validateClusterAdminRoleForSA(client)) + if err := o.validateClusterAdminRoleForSA(client, result); err != nil { + return err + } } - return result + return nil } -func (o kudoServiceAccount) Install(client *kube.Client) error { +func (o KudoServiceAccount) Install(client *kube.Client) error { if !o.opts.IsDefaultServiceAccount() { return nil } @@ -61,41 +67,50 @@ func (o kudoServiceAccount) Install(client *kube.Client) error { return nil } +func (o KudoServiceAccount) Resources() []runtime.Object { + if o.opts.IsDefaultServiceAccount() { + return []runtime.Object{o.serviceAccount, o.roleBinding} + } + return make([]runtime.Object, 0) +} + // Validate whether the serviceAccount exists -func (o kudoServiceAccount) validateServiceAccountExists(client *kube.Client) verifier.Result { +func (o KudoServiceAccount) validateServiceAccountExists(client *kube.Client, result *verifier.Result) error { coreClient := client.KubeClient.CoreV1() saList, err := coreClient.ServiceAccounts(o.opts.Namespace).List(metav1.ListOptions{}) if err != nil { - return verifier.NewError(fmt.Sprintf("Failed to validate that service account %s exists: %v", o.opts.ServiceAccount, err)) + return fmt.Errorf("failed to retrieve list of service accounts from namespace %v: %v", o.opts.Namespace, err) } for _, sa := range saList.Items { if sa.Name == o.opts.ServiceAccount { - return verifier.NewResult() + return nil } } - return verifier.NewError(fmt.Sprintf("Service Account %s does not exists - KUDO expects the serviceAccount to be present in the namespace %s", o.opts.ServiceAccount, o.opts.Namespace)) + result.AddErrors(fmt.Sprintf("Service Account %s does not exists - KUDO expects the serviceAccount to be present in the namespace %s", o.opts.ServiceAccount, o.opts.Namespace)) + return nil } // Validate whether the serviceAccount has cluster-admin role -func (o kudoServiceAccount) validateClusterAdminRoleForSA(client *kube.Client) verifier.Result { +func (o KudoServiceAccount) validateClusterAdminRoleForSA(client *kube.Client, result *verifier.Result) error { // Check whether the serviceAccount has clusterrolebinding cluster-admin crbs, err := client.KubeClient.RbacV1().ClusterRoleBindings().List(metav1.ListOptions{}) if err != nil { - return verifier.NewError(fmt.Sprintf("Failed to validate role bindings: %v", err)) + return fmt.Errorf("failed to retrieve list of role bindings: %v", err) } for _, crb := range crbs.Items { for _, subject := range crb.Subjects { if subject.Name == o.opts.ServiceAccount && subject.Namespace == o.opts.Namespace && crb.RoleRef.Name == "cluster-admin" { - return verifier.NewResult() + return nil } } } - return verifier.NewError(fmt.Sprintf("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", o.opts.ServiceAccount, o.opts.Namespace)) + result.AddErrors(fmt.Sprintf("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", o.opts.ServiceAccount, o.opts.Namespace)) + return nil } -func (o kudoServiceAccount) installServiceAccount(client *kube.Client) error { +func (o KudoServiceAccount) installServiceAccount(client *kube.Client) error { coreClient := client.KubeClient.CoreV1() _, err := coreClient.ServiceAccounts(o.opts.Namespace).Create(o.serviceAccount) if kerrors.IsAlreadyExists(err) { @@ -105,7 +120,7 @@ func (o kudoServiceAccount) installServiceAccount(client *kube.Client) error { return err } -func (o kudoServiceAccount) installRoleBinding(client *kube.Client) error { +func (o KudoServiceAccount) installRoleBinding(client *kube.Client) error { _, err := client.KubeClient.RbacV1().ClusterRoleBindings().Create(o.roleBinding) if kerrors.IsAlreadyExists(err) { clog.V(4).Printf("role binding %v already exists", o.roleBinding.Name) @@ -114,36 +129,6 @@ func (o kudoServiceAccount) installRoleBinding(client *kube.Client) error { return err } -func (o kudoServiceAccount) ValidateInstallation(client *kube.Client) error { - coreClient := client.KubeClient.CoreV1() - - existingSA, err := coreClient.ServiceAccounts(o.opts.Namespace).Get(o.serviceAccount.Name, metav1.GetOptions{}) - if err != nil { - return fmt.Errorf("failed to retrieve service account %v", err) - } - - if !reflect.DeepEqual(existingSA, o.serviceAccount) { - return fmt.Errorf("installed ServiceAccount does not equal expected service account") - } - - existingRB, err := client.KubeClient.RbacV1().RoleBindings(o.opts.Namespace).Get(o.roleBinding.Name, metav1.GetOptions{}) - if err != nil { - return fmt.Errorf("failed to retrieve role binding %v", err) - } - - if !reflect.DeepEqual(existingRB, o.roleBinding) { - return fmt.Errorf("installed ClusterRoleBinding does not equal expected") - } - return nil -} - -func (o kudoServiceAccount) AsRuntimeObjs() []runtime.Object { - if o.opts.IsDefaultServiceAccount() { - return []runtime.Object{o.serviceAccount, o.roleBinding} - } - return make([]runtime.Object, 0) -} - // generateServiceAccount builds the system account func generateServiceAccount(opts kudoinit.Options) *v1.ServiceAccount { labels := kudoinit.GenerateLabels(map[string]string{}) diff --git a/pkg/kudoctl/kudoinit/prereq/serviceaccount_test.go b/pkg/kudoctl/kudoinit/prereq/serviceaccount_test.go index db585251d..c7c4d1587 100644 --- a/pkg/kudoctl/kudoinit/prereq/serviceaccount_test.go +++ b/pkg/kudoctl/kudoinit/prereq/serviceaccount_test.go @@ -3,8 +3,16 @@ package prereq import ( "testing" + v1 "k8s.io/api/core/v1" + "github.com/stretchr/testify/assert" + rbac "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + testing2 "k8s.io/client-go/testing" + "github.com/kudobuilder/kudo/pkg/kudoctl/kube" "github.com/kudobuilder/kudo/pkg/kudoctl/kudoinit" "github.com/kudobuilder/kudo/pkg/kudoctl/verifier" ) @@ -12,9 +20,10 @@ import ( func TestPrereq_Fail_PreValidate_CustomServiceAccount(t *testing.T) { client := getFakeClient() - init := NewInitializer(kudoinit.NewOptions("", "", "customSA", make([]string, 0), false)) + init := NewServiceAccountInitializer(kudoinit.NewOptions("", "", "customSA", make([]string, 0), false)) - result := init.PreInstallVerify(client) + result := verifier.NewResult() + _ = init.PreInstallVerify(client, &result) assert.EqualValues(t, verifier.NewError("Service Account customSA does not exists - KUDO expects the serviceAccount to be present in the namespace kudo-system"), result) } @@ -26,9 +35,10 @@ func TestPrereq_Fail_PreValidate_CustomServiceAccount_MissingPermissions(t *test mockListServiceAccounts(client, customSA) - init := NewInitializer(kudoinit.NewOptions("", "", customSA, make([]string, 0), false)) + init := NewServiceAccountInitializer(kudoinit.NewOptions("", "", "customSA", make([]string, 0), false)) - result := init.PreInstallVerify(client) + result := verifier.NewResult() + _ = init.PreInstallVerify(client, &result) assert.EqualValues(t, verifier.NewError("Service Account customSA does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace kudo-system and to have cluster-admin role"), result) } @@ -42,8 +52,47 @@ func TestPrereq_Ok_PreValidate_CustomServiceAccount(t *testing.T) { mockListServiceAccounts(client, opts.ServiceAccount) mockListClusterRoleBindings(client, opts) - init := NewInitializer(opts) - result := init.PreInstallVerify(client) + init := NewServiceAccountInitializer(opts) + result := verifier.NewResult() + _ = init.PreInstallVerify(client, &result) assert.EqualValues(t, verifier.NewResult(), result) } + +func mockListServiceAccounts(client *kube.Client, saName string) { + client.KubeClient.(*fake.Clientset).Fake.PrependReactor("list", "serviceaccounts", func(action testing2.Action) (handled bool, ret runtime.Object, err error) { + sa := v1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: saName, + }, + Secrets: nil, + ImagePullSecrets: nil, + AutomountServiceAccountToken: nil, + } + sal := &v1.ServiceAccountList{ + Items: []v1.ServiceAccount{sa}, + } + return true, sal, nil + }) +} + +func mockListClusterRoleBindings(client *kube.Client, opts kudoinit.Options) { + client.KubeClient.(*fake.Clientset).Fake.PrependReactor("list", "clusterrolebindings", func(action testing2.Action) (handled bool, ret runtime.Object, err error) { + subject := rbac.Subject{ + Kind: "", + Name: opts.ServiceAccount, + Namespace: opts.Namespace, + } + + crb := rbac.ClusterRoleBinding{ + Subjects: []rbac.Subject{subject}, + RoleRef: rbac.RoleRef{ + Name: "cluster-admin", + }, + } + crbList := &rbac.ClusterRoleBindingList{ + Items: []rbac.ClusterRoleBinding{crb}, + } + return true, crbList, nil + }) +} diff --git a/pkg/kudoctl/kudoinit/prereq/webhook.go b/pkg/kudoctl/kudoinit/prereq/webhook.go index 473a821f9..813488e3f 100644 --- a/pkg/kudoctl/kudoinit/prereq/webhook.go +++ b/pkg/kudoctl/kudoinit/prereq/webhook.go @@ -25,10 +25,12 @@ import ( ) // Ensure IF is implemented -var _ k8sResource = &kudoWebHook{} +var _ kudoinit.Step = &KudoWebHook{} -type kudoWebHook struct { - opts kudoinit.Options +type KudoWebHook struct { + opts kudoinit.Options + issuer unstructured.Unstructured + certificate unstructured.Unstructured } const ( @@ -40,33 +42,31 @@ var ( certManagerControllerImageSuffix = fmt.Sprintf("cert-manager-controller:%s", certManagerControllerVersion) ) -func newWebHook(options kudoinit.Options) kudoWebHook { - return kudoWebHook{ - opts: options, +func NewWebHookInitializer(options kudoinit.Options) KudoWebHook { + return KudoWebHook{ + opts: options, + certificate: certificate(options.Namespace), + issuer: issuer(options.Namespace), } } -func (k kudoWebHook) PreInstallVerify(client *kube.Client) verifier.Result { - // skip verification if webhooks are not used or self-signed CA is used - if !k.opts.HasWebhooksEnabled() || k.opts.SelfSignedWebhookCA { - return verifier.NewResult() - } - return validateCertManagerInstallation(client) +func (k KudoWebHook) String() string { + return "webhook" } -func (k kudoWebHook) Install(client *kube.Client) error { - if !k.opts.HasWebhooksEnabled() { +func (k KudoWebHook) PreInstallVerify(client *kube.Client, result *verifier.Result) error { + // skip verification if webhooks are not used or self-signed CA is used + if !k.opts.HasWebhooksEnabled() || k.opts.SelfSignedWebhookCA { return nil } - - if k.opts.SelfSignedWebhookCA { - return k.installWithSelfSignedCA(client) - } - return k.installWithCertManager(client) + return validateCertManagerInstallation(client, result) } -func (k kudoWebHook) installWithCertManager(client *kube.Client) error { - if err := installUnstructured(client.DynamicClient, certificate(k.opts.Namespace)); err != nil { +func (k KudoWebHook) installWithCertManager(client *kube.Client) error { + if err := installUnstructured(client.DynamicClient, k.issuer); err != nil { + return err + } + if err := installUnstructured(client.DynamicClient, k.certificate); err != nil { return err } if err := installAdmissionWebhook(client.KubeClient.AdmissionregistrationV1beta1(), InstanceAdmissionWebhook(k.opts.Namespace)); err != nil { @@ -75,7 +75,7 @@ func (k kudoWebHook) installWithCertManager(client *kube.Client) error { return nil } -func (k kudoWebHook) installWithSelfSignedCA(client *kube.Client) error { +func (k KudoWebHook) installWithSelfSignedCA(client *kube.Client) error { iaw, s, err := k.runtimeObjsWithSelfSignedCA() if err != nil { return nil @@ -92,16 +92,17 @@ func (k kudoWebHook) installWithSelfSignedCA(client *kube.Client) error { return nil } -func (k kudoWebHook) ValidateInstallation(client *kube.Client) error { +func (k KudoWebHook) Install(client *kube.Client) error { if !k.opts.HasWebhooksEnabled() { return nil } - - // TODO: Check installed webhooks, check if cert-manager is installed, etc - panic("implement me") + if k.opts.SelfSignedWebhookCA { + return k.installWithSelfSignedCA(client) + } + return k.installWithCertManager(client) } -func (k kudoWebHook) AsRuntimeObjs() []runtime.Object { +func (k KudoWebHook) Resources() []runtime.Object { if !k.opts.HasWebhooksEnabled() { return make([]runtime.Object, 0) } @@ -118,18 +119,15 @@ func (k kudoWebHook) AsRuntimeObjs() []runtime.Object { return k.runtimeObjsWithCertManager() } -func (k kudoWebHook) runtimeObjsWithCertManager() []runtime.Object { - iaw := InstanceAdmissionWebhook(k.opts.Namespace) - certObjs := certificate(k.opts.Namespace) - objs := []runtime.Object{&iaw} - for _, c := range certObjs { - c := c - objs = append(objs, &c) - } +func (k KudoWebHook) runtimeObjsWithCertManager() []runtime.Object { + av := InstanceAdmissionWebhook(k.opts.Namespace) + objs := []runtime.Object{&av} + objs = append(objs, &k.issuer) + objs = append(objs, &k.certificate) return objs } -func (k kudoWebHook) runtimeObjsWithSelfSignedCA() (*admissionv1beta1.MutatingWebhookConfiguration, *corev1.Secret, error) { +func (k KudoWebHook) runtimeObjsWithSelfSignedCA() (*admissionv1beta1.MutatingWebhookConfiguration, *corev1.Secret, error) { tinyCA, err := NewTinyCA(kudoinit.DefaultServiceName, k.opts.Namespace) if err != nil { return nil, nil, fmt.Errorf("unable to set up webhook CA: %v", err) @@ -151,64 +149,71 @@ func (k kudoWebHook) runtimeObjsWithSelfSignedCA() (*admissionv1beta1.MutatingWe return &iaw, &ws, nil } -func validateCertManagerInstallation(client *kube.Client) verifier.Result { - result := verifier.NewResult() - result.Merge(validateCrdVersion(client.ExtClient, "certificates.cert-manager.io", certManagerAPIVersion)) - result.Merge(validateCrdVersion(client.ExtClient, "issuers.cert-manager.io", certManagerAPIVersion)) +func validateCertManagerInstallation(client *kube.Client, result *verifier.Result) error { + if err := validateCrdVersion(client.ExtClient, "certificates.cert-manager.io", certManagerAPIVersion, result); err != nil { + return err + } + if err := validateCrdVersion(client.ExtClient, "issuers.cert-manager.io", certManagerAPIVersion, result); err != nil { + return err + } if !result.IsEmpty() { - return result + // Abort verify here, if we don't have CRDs the remaining checks don't make much sense + return nil } deployment, err := client.KubeClient.AppsV1().Deployments("cert-manager").Get("cert-manager", metav1.GetOptions{}) if err != nil { - return verifier.NewWarning(fmt.Sprintf("failed to get cert-manager deployment in namespace cert-manager. Make sure cert-manager is running (%s)", err)) + if kerrors.IsNotFound(err) { + result.AddWarnings(fmt.Sprintf("failed to get cert-manager deployment in namespace cert-manager. Make sure cert-manager is running.")) + return nil + } + return err } if len(deployment.Spec.Template.Spec.Containers) < 1 { - return verifier.NewWarning("failed to validate cert-manager controller deployment. Spec had no containers") + result.AddWarnings("failed to validate cert-manager controller deployment. Spec had no containers") + return nil } if !strings.HasSuffix(deployment.Spec.Template.Spec.Containers[0].Image, certManagerControllerImageSuffix) { - return verifier.NewWarning(fmt.Sprintf("cert-manager deployment had unexpected version. expected %s in controller image name but found %s", certManagerControllerVersion, deployment.Spec.Template.Spec.Containers[0].Image)) + result.AddWarnings(fmt.Sprintf("cert-manager deployment had unexpected version. expected %s in controller image name but found %s", certManagerControllerVersion, deployment.Spec.Template.Spec.Containers[0].Image)) + return nil } - if err := health.IsHealthy(deployment); err != nil { - return verifier.NewWarning("cert-manager seems not to be running correctly. Make sure cert-manager is working") + result.AddWarnings("cert-manager seems not to be running correctly. Make sure cert-manager is working") + return nil } - - return verifier.NewResult() + return nil } -func validateCrdVersion(extClient clientset.Interface, crdName string, expectedVersion string) verifier.Result { +func validateCrdVersion(extClient clientset.Interface, crdName string, expectedVersion string, result *verifier.Result) error { certCRD, err := extClient.ApiextensionsV1().CustomResourceDefinitions().Get(crdName, metav1.GetOptions{}) if err != nil { if kerrors.IsNotFound(err) { - return verifier.NewError(fmt.Sprintf("failed to find CRD '%s': %s", crdName, err)) + result.AddErrors(fmt.Sprintf("failed to find CRD '%s': %s", crdName, err)) + return nil } - return verifier.NewError(fmt.Sprintf("Failed to retrieve CRD '%s': %s", crdName, err)) + return err } crdVersion := certCRD.Spec.Versions[0].Name if crdVersion != expectedVersion { - return verifier.NewError(fmt.Sprintf("invalid CRD version found for '%s': %s instead of %s", crdName, crdVersion, expectedVersion)) + result.AddErrors(fmt.Sprintf("invalid CRD version found for '%s': %s instead of %s", crdName, crdVersion, expectedVersion)) } - return verifier.NewResult() + return nil } // installUnstructured accepts kubernetes resource as unstructured.Unstructured and installs it into cluster -func installUnstructured(dynamicClient dynamic.Interface, items []unstructured.Unstructured) error { - for _, item := range items { - obj := item - gvk := item.GroupVersionKind() - _, err := dynamicClient.Resource(schema.GroupVersionResource{ - Group: gvk.Group, - Version: gvk.Version, - Resource: fmt.Sprintf("%ss", strings.ToLower(gvk.Kind)), // since we know what kinds are we dealing with here, this is OK - }).Namespace(obj.GetNamespace()).Create(&obj, metav1.CreateOptions{}) - if kerrors.IsAlreadyExists(err) { - clog.V(4).Printf("resource %s already registered", obj.GetName()) - } else if err != nil { - return fmt.Errorf("failed to create resource %s/%s: %v", obj.GetName(), obj.GetNamespace(), err) - } +func installUnstructured(dynamicClient dynamic.Interface, item unstructured.Unstructured) error { + gvk := item.GroupVersionKind() + _, err := dynamicClient.Resource(schema.GroupVersionResource{ + Group: gvk.Group, + Version: gvk.Version, + Resource: fmt.Sprintf("%ss", strings.ToLower(gvk.Kind)), // since we know what kinds are we dealing with here, this is OK + }).Namespace(item.GetNamespace()).Create(&item, metav1.CreateOptions{}) + if kerrors.IsAlreadyExists(err) { + clog.V(4).Printf("resource %s already registered", item.GetName()) + } else if err != nil { + return fmt.Errorf("error when creating resource %s/%s. %v", item.GetName(), item.GetNamespace(), err) } return nil } @@ -283,38 +288,39 @@ func InstanceAdmissionWebhook(ns string) admissionv1beta1.MutatingWebhookConfigu } } -func certificate(ns string) []unstructured.Unstructured { - return []unstructured.Unstructured{ - { - Object: map[string]interface{}{ - "apiVersion": "cert-manager.io/v1alpha2", - "kind": "Issuer", - "metadata": map[string]interface{}{ - "name": "selfsigned-issuer", - "namespace": ns, - }, - "spec": map[string]interface{}{ - "selfSigned": map[string]interface{}{}, - }, +func issuer(ns string) unstructured.Unstructured { + return unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "cert-manager.io/v1alpha2", + "kind": "Issuer", + "metadata": map[string]interface{}{ + "name": "selfsigned-issuer", + "namespace": ns, + }, + "spec": map[string]interface{}{ + "selfSigned": map[string]interface{}{}, }, }, - { - Object: map[string]interface{}{ - "apiVersion": "cert-manager.io/v1alpha2", - "kind": "Certificate", - "metadata": map[string]interface{}{ - "name": "kudo-webhook-server-certificate", - "namespace": ns, - }, - "spec": map[string]interface{}{ - "commonName": "kudo-controller-manager-service.kudo-system.svc", - "dnsNames": []string{"kudo-controller-manager-service.kudo-system.svc"}, - "issuerRef": map[string]interface{}{ - "kind": "Issuer", - "name": "selfsigned-issuer", - }, - "secretName": kudoinit.DefaultSecretName, + } +} + +func certificate(ns string) unstructured.Unstructured { + return unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "cert-manager.io/v1alpha2", + "kind": "Certificate", + "metadata": map[string]interface{}{ + "name": "kudo-webhook-server-certificate", + "namespace": ns, + }, + "spec": map[string]interface{}{ + "commonName": "kudo-controller-manager-service.kudo-system.svc", + "dnsNames": []string{"kudo-controller-manager-service.kudo-system.svc"}, + "issuerRef": map[string]interface{}{ + "kind": "Issuer", + "name": "selfsigned-issuer", }, + "secretName": kudoinit.DefaultSecretName, }, }, } diff --git a/pkg/kudoctl/kudoinit/prereq/webhook_test.go b/pkg/kudoctl/kudoinit/prereq/webhook_test.go index 88ac8f042..194ce9c9c 100644 --- a/pkg/kudoctl/kudoinit/prereq/webhook_test.go +++ b/pkg/kudoctl/kudoinit/prereq/webhook_test.go @@ -18,9 +18,10 @@ import ( func TestPrereq_Ok_PreValidate_Webhook_None(t *testing.T) { client := getFakeClient() - init := NewInitializer(kudoinit.NewOptions("", "", "", []string{}, false)) + init := NewWebHookInitializer(kudoinit.NewOptions("", "", "", []string{}, false)) - result := init.PreInstallVerify(client) + result := verifier.NewResult() + _ = init.PreInstallVerify(client, &result) assert.EqualValues(t, verifier.NewResult(), result) } @@ -28,9 +29,10 @@ func TestPrereq_Ok_PreValidate_Webhook_None(t *testing.T) { func TestPrereq_Fail_PreValidate_Webhook_NoCertificate(t *testing.T) { client := getFakeClient() - init := NewInitializer(kudoinit.NewOptions("", "", "", []string{"validate"}, false)) + init := NewWebHookInitializer(kudoinit.NewOptions("", "", "", []string{"validate"}, false)) - result := init.PreInstallVerify(client) + result := verifier.NewResult() + _ = init.PreInstallVerify(client, &result) assert.EqualValues(t, verifier.NewError( "failed to find CRD 'certificates.cert-manager.io': customresourcedefinitions.apiextensions.k8s.io \"certificates.cert-manager.io\" not found", @@ -44,9 +46,10 @@ func TestPrereq_Fail_PreValidate_Webhook_WrongCertificateVersion(t *testing.T) { mockCRD(client, "certificates.cert-manager.io", "v0") mockCRD(client, "issuers.cert-manager.io", "v0") - init := NewInitializer(kudoinit.NewOptions("", "", "", []string{"validate"}, false)) + init := NewWebHookInitializer(kudoinit.NewOptions("", "", "", []string{"validate"}, false)) - result := init.PreInstallVerify(client) + result := verifier.NewResult() + _ = init.PreInstallVerify(client, &result) assert.EqualValues(t, verifier.NewError( "invalid CRD version found for 'certificates.cert-manager.io': v0 instead of v1alpha2", @@ -59,9 +62,10 @@ func TestPrereq_Fail_PreValidate_Webhook_NoIssuer(t *testing.T) { mockCRD(client, "certificates.cert-manager.io", "v1alpha2") - init := NewInitializer(kudoinit.NewOptions("", "", "", []string{"validate"}, false)) + init := NewWebHookInitializer(kudoinit.NewOptions("", "", "", []string{"validate"}, false)) - result := init.PreInstallVerify(client) + result := verifier.NewResult() + _ = init.PreInstallVerify(client, &result) assert.EqualValues(t, verifier.NewError("failed to find CRD 'issuers.cert-manager.io': customresourcedefinitions.apiextensions.k8s.io \"issuers.cert-manager.io\" not found"), result) } @@ -72,9 +76,10 @@ func TestPrereq_Fail_PreValidate_Webhook_WrongIssuerVersion(t *testing.T) { mockCRD(client, "certificates.cert-manager.io", "v1alpha2") mockCRD(client, "issuers.cert-manager.io", "v0") - init := NewInitializer(kudoinit.NewOptions("", "", "", []string{"validate"}, false)) + init := NewWebHookInitializer(kudoinit.NewOptions("", "", "", []string{"validate"}, false)) - result := init.PreInstallVerify(client) + result := verifier.NewResult() + _ = init.PreInstallVerify(client, &result) assert.EqualValues(t, verifier.NewError("invalid CRD version found for 'issuers.cert-manager.io': v0 instead of v1alpha2"), result) } @@ -85,9 +90,10 @@ func TestPrereq_Ok_PreValidate_Webhook(t *testing.T) { mockCRD(client, "certificates.cert-manager.io", "v1alpha2") mockCRD(client, "issuers.cert-manager.io", "v1alpha2") - init := NewInitializer(kudoinit.NewOptions("", "", "", []string{"validate"}, false)) + init := NewWebHookInitializer(kudoinit.NewOptions("", "", "", []string{"validate"}, false)) - result := init.PreInstallVerify(client) + result := verifier.NewResult() + _ = init.PreInstallVerify(client, &result) assert.Equal(t, 0, len(result.Errors)) } diff --git a/pkg/kudoctl/kudoinit/setup/setup.go b/pkg/kudoctl/kudoinit/setup/setup.go index bbde0c551..ee3ec44ad 100644 --- a/pkg/kudoctl/kudoinit/setup/setup.go +++ b/pkg/kudoctl/kudoinit/setup/setup.go @@ -1,9 +1,7 @@ package setup import ( - "errors" "fmt" - "os" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/yaml" @@ -17,23 +15,24 @@ import ( "github.com/kudobuilder/kudo/pkg/kudoctl/verifier" ) -// Install uses Kubernetes client to install KUDO. -func Install(client *kube.Client, opts kudoinit.Options, crdOnly bool) error { +// Verifies that the installation is possible. Returns an error if any part of KUDO is already installed +func PreInstallVerify(client *kube.Client, opts kudoinit.Options, crdOnly bool, result *verifier.Result) error { initSteps := initSteps(opts, crdOnly) - result := verifier.NewResult() // Check if all steps are installable for _, initStep := range initSteps { - result.Merge(initStep.PreInstallVerify(client)) + if err := initStep.PreInstallVerify(client, result); err != nil { + return fmt.Errorf("error while verifying install step %s: %v", initStep.String(), err) + } } - result.PrintWarnings(os.Stdout) - if !result.IsValid() { - result.PrintErrors(os.Stdout) - return errors.New(result.ErrorsAsString()) - } + return nil +} +// Install uses Kubernetes client to install KUDO. +func Install(client *kube.Client, opts kudoinit.Options, crdOnly bool) error { // Install everything + initSteps := initSteps(opts, crdOnly) for _, initStep := range initSteps { if err := initStep.Install(client); err != nil { return fmt.Errorf("%s: %v", initStep, err) @@ -53,7 +52,9 @@ func initSteps(opts kudoinit.Options, crdOnly bool) []kudoinit.Step { return []kudoinit.Step{ crd.NewInitializer(), - prereq.NewInitializer(opts), + prereq.NewNamespaceInitializer(opts), + prereq.NewServiceAccountInitializer(opts), + prereq.NewWebHookInitializer(opts), manager.NewInitializer(opts), } } diff --git a/pkg/kudoctl/kudoinit/types.go b/pkg/kudoctl/kudoinit/types.go index d517af6b9..5c69fd630 100644 --- a/pkg/kudoctl/kudoinit/types.go +++ b/pkg/kudoctl/kudoinit/types.go @@ -25,10 +25,7 @@ type Artifacter interface { type InstallVerifier interface { // PreInstallVerify verifies that the installation is possible - PreInstallVerify(client *kube.Client) verifier.Result - - // TODO: Add verification of existing installation - // VerifyInstallation(client *kube.Client) Result + PreInstallVerify(client *kube.Client, result *verifier.Result) error } type Installer interface { diff --git a/pkg/kudoctl/util/kudo/kudo.go b/pkg/kudoctl/util/kudo/kudo.go index 859376a62..5993cc8f7 100644 --- a/pkg/kudoctl/util/kudo/kudo.go +++ b/pkg/kudoctl/util/kudo/kudo.go @@ -25,6 +25,7 @@ import ( "github.com/kudobuilder/kudo/pkg/kudoctl/clog" "github.com/kudobuilder/kudo/pkg/kudoctl/kube" "github.com/kudobuilder/kudo/pkg/kudoctl/kudoinit/crd" + "github.com/kudobuilder/kudo/pkg/kudoctl/verifier" "github.com/kudobuilder/kudo/pkg/util/convert" label "github.com/kudobuilder/kudo/pkg/util/kudo" "github.com/kudobuilder/kudo/pkg/version" @@ -52,15 +53,15 @@ func NewClient(kubeConfigPath string, requestTimeout int64, validateInstall bool return nil, clog.Errorf("could not get Kubernetes client: %s", err) } - err = crd.NewInitializer().ValidateInstallation(kubeClient) - if err != nil { - // see above - if os.IsTimeout(err) { - return nil, err - } + result := verifier.NewResult() + err = crd.NewInitializer().VerifyInstallation(kubeClient, &result) + if err != nil || !result.IsValid() { clog.V(0).Printf("KUDO CRDs are not set up correctly. Do you need to run kudo init?") if validateInstall { - return nil, fmt.Errorf("CRDs invalid: %v", err) + if err != nil { + return nil, fmt.Errorf("CRDs invalid: %v", err) + } + return nil, fmt.Errorf("CRDs invalid: %v", result.ErrorsAsString()) } } From 83905e8388db1b9e537de23c5d76b8a958ae2fbc Mon Sep 17 00:00:00 2001 From: Andreas Neumann Date: Mon, 4 May 2020 16:18:45 +0200 Subject: [PATCH 2/4] Small review requests Signed-off-by: Andreas Neumann --- pkg/kudoctl/kudoinit/crd/crds.go | 6 +++--- pkg/kudoctl/kudoinit/prereq/namespace_test.go | 8 +++----- pkg/kudoctl/util/kudo/kudo.go | 9 +++++---- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/pkg/kudoctl/kudoinit/crd/crds.go b/pkg/kudoctl/kudoinit/crd/crds.go index d86301344..1fecfeceb 100644 --- a/pkg/kudoctl/kudoinit/crd/crds.go +++ b/pkg/kudoctl/kudoinit/crd/crds.go @@ -7,7 +7,7 @@ import ( apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" + kerrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/yaml" @@ -83,7 +83,7 @@ func (c Initializer) verifyInstallation(client v1beta1.CustomResourceDefinitions if os.IsTimeout(err) { return err } - if k8serrors.IsNotFound(err) { + if kerrors.IsNotFound(err) { result.AddErrors(fmt.Sprintf("CRD %s is not installed", crd.Name)) return nil } @@ -98,7 +98,7 @@ func (c Initializer) verifyInstallation(client v1beta1.CustomResourceDefinitions func (c Initializer) install(client v1beta1.CustomResourceDefinitionsGetter, crd *apiextv1beta1.CustomResourceDefinition) error { _, err := client.CustomResourceDefinitions().Create(crd) - if k8serrors.IsAlreadyExists(err) { + if kerrors.IsAlreadyExists(err) { clog.V(4).Printf("crd %v already exists", crd.Name) return nil } diff --git a/pkg/kudoctl/kudoinit/prereq/namespace_test.go b/pkg/kudoctl/kudoinit/prereq/namespace_test.go index fd9c1afd2..26268c22e 100644 --- a/pkg/kudoctl/kudoinit/prereq/namespace_test.go +++ b/pkg/kudoctl/kudoinit/prereq/namespace_test.go @@ -3,16 +3,14 @@ package prereq import ( "testing" + "github.com/stretchr/testify/assert" core "k8s.io/api/core/v1" - v12 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" testing2 "k8s.io/client-go/testing" "github.com/kudobuilder/kudo/pkg/kudoctl/kube" - - "github.com/stretchr/testify/assert" - "github.com/kudobuilder/kudo/pkg/kudoctl/kudoinit" "github.com/kudobuilder/kudo/pkg/kudoctl/verifier" ) @@ -43,7 +41,7 @@ func TestPrereq_Ok_PreValidate_CustomNamespace(t *testing.T) { func mockGetNamespace(client *kube.Client, nsName string) { client.KubeClient.(*fake.Clientset).Fake.PrependReactor("get", "namespaces", func(action testing2.Action) (handled bool, ret runtime.Object, err error) { ns := &core.Namespace{ - ObjectMeta: v12.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: nsName, }, } diff --git a/pkg/kudoctl/util/kudo/kudo.go b/pkg/kudoctl/util/kudo/kudo.go index 5993cc8f7..4c2587d89 100644 --- a/pkg/kudoctl/util/kudo/kudo.go +++ b/pkg/kudoctl/util/kudo/kudo.go @@ -55,12 +55,13 @@ func NewClient(kubeConfigPath string, requestTimeout int64, validateInstall bool result := verifier.NewResult() err = crd.NewInitializer().VerifyInstallation(kubeClient, &result) - if err != nil || !result.IsValid() { + if err != nil { + return nil, fmt.Errorf("failed to run crd verification: %v", err) + } + if !result.IsValid() { clog.V(0).Printf("KUDO CRDs are not set up correctly. Do you need to run kudo init?") + if validateInstall { - if err != nil { - return nil, fmt.Errorf("CRDs invalid: %v", err) - } return nil, fmt.Errorf("CRDs invalid: %v", result.ErrorsAsString()) } } From 8c55057572db62181d780333bde0ddc6164c422e Mon Sep 17 00:00:00 2001 From: Andreas Neumann Date: Mon, 4 May 2020 16:47:17 +0200 Subject: [PATCH 3/4] Fixed unit test Signed-off-by: Andreas Neumann --- pkg/kudoctl/util/kudo/kudo_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kudoctl/util/kudo/kudo_test.go b/pkg/kudoctl/util/kudo/kudo_test.go index 8c2d0acbd..2c57a05bc 100644 --- a/pkg/kudoctl/util/kudo/kudo_test.go +++ b/pkg/kudoctl/util/kudo/kudo_test.go @@ -23,7 +23,7 @@ func TestKudoClientValidate(t *testing.T) { tests := []struct { err string }{ - {"CRDs invalid: failed to retrieve CRD"}, // verify that NewClient tries to validate CRDs + {"failed to run crd verification: failed to retrieve CRD"}, // verify that NewClient tries to validate CRDs } for _, tt := range tests { From 6a8cf4ecc829b9e3629b043b42b6dc1c7b3af64c Mon Sep 17 00:00:00 2001 From: Andreas Neumann Date: Mon, 4 May 2020 18:11:25 +0200 Subject: [PATCH 4/4] Review requests Signed-off-by: Andreas Neumann --- pkg/kudoctl/cmd/init_integration_test.go | 202 +++++++++++------------ pkg/kudoctl/kudoinit/prereq/webhook.go | 10 +- 2 files changed, 101 insertions(+), 111 deletions(-) diff --git a/pkg/kudoctl/cmd/init_integration_test.go b/pkg/kudoctl/cmd/init_integration_test.go index cb32dccbe..b6b88b5ba 100644 --- a/pkg/kudoctl/cmd/init_integration_test.go +++ b/pkg/kudoctl/cmd/init_integration_test.go @@ -192,115 +192,105 @@ func TestIntegInitWithNameSpace(t *testing.T) { 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 := crd.NewInitializer().Resources() - defer deleteInitObjects(testClient) - - var buf bytes.Buffer - cmd := &initCmd{ - out: &buf, - fs: afero.NewMemMapFs(), - client: kclient, - ns: namespace, - serviceAccount: "test-account", +func TestInitWithServiceAccount(t *testing.T) { + tests := []struct { + name string + serviceAccount string + roleBindingRole string + roleBindingNs string + errMessageContains string + }{ + {name: "service account not present", serviceAccount: "", errMessageContains: "Service Account test-account does not exists - KUDO expects the serviceAccount to be present in the namespace sa-integration-test-0"}, + {name: "service account has no rb", serviceAccount: "test-account", errMessageContains: "Service Account test-account does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace sa-integration-test-1 and to have cluster-admin role"}, + {name: "rb has no cluster-admin role", serviceAccount: "test-account", roleBindingRole: "not-admin", errMessageContains: "Service Account test-account does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace sa-integration-test-2 and to have cluster-admin role"}, + {name: "rb has different ns", serviceAccount: "test-account", roleBindingRole: "not-admin", roleBindingNs: "otherns", errMessageContains: "Service Account test-account does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace sa-integration-test-3 and to have cluster-admin role"}, + {name: "rb has admin in different ns", serviceAccount: "test-account", roleBindingRole: "cluster-admin", roleBindingNs: "otherns", errMessageContains: "Service Account test-account does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace sa-integration-test-4 and to have cluster-admin role"}, + {name: "rb has cluster-admin role", serviceAccount: "test-account", roleBindingRole: "cluster-admin", errMessageContains: ""}, } - // 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, "failed to verify installation requirements", err.Error()) - assertStringContains(t, "Service Account test-account does not exists - KUDO expects the serviceAccount to be present in the namespace sa-integration-test", buf.String()) - - // 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, "failed to verify installation requirements", err.Error()) - assertStringContains(t, "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", buf.String()) - - // 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, "failed to verify installation requirements", err.Error()) - assertStringContains(t, "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", buf.String()) - - // 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, "failed to verify installation requirements", err.Error()) - assertStringContains(t, "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", buf.String()) - - // 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 == kudoinit.DefaultManagerName { - kudoControllerFound = true - } + namespaceBase := "sa-integration-test" + + for idx, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + namespace := fmt.Sprintf("%s-%d", namespaceBase, idx) + + // 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 := crd.NewInitializer().Resources() + defer deleteInitObjects(testClient) + + var buf bytes.Buffer + cmd := &initCmd{ + out: &buf, + fs: afero.NewMemMapFs(), + client: kclient, + ns: namespace, + serviceAccount: "test-account", + } + + ns := testutils.NewResource("v1", "Namespace", namespace, "") + assert.NoError(t, testClient.Create(context.TODO(), ns)) + defer testClient.Delete(context.TODO(), ns) + + if tt.serviceAccount != "" { + sa2 := testutils.NewResource("v1", "ServiceAccount", tt.serviceAccount, namespace) + assert.NoError(t, testClient.Create(context.TODO(), sa2)) + defer testClient.Delete(context.TODO(), sa2) + } + + if tt.roleBindingRole != "" { + rbNamespace := tt.roleBindingNs + if rbNamespace == "" { + rbNamespace = namespace + } + crb := testutils.NewClusterRoleBinding("rbac.authorization.k8s.io/v1", "ClusterRoleBinding", "kudo-clusterrole-binding", rbNamespace, tt.serviceAccount, tt.roleBindingRole) + assert.NoError(t, testClient.Create(context.TODO(), crb)) + defer testClient.Delete(context.TODO(), crb) + } + + err = cmd.run() + + if tt.errMessageContains != "" { + require.Error(t, err) + assert.Equal(t, "failed to verify installation requirements", err.Error()) + assertStringContains(t, tt.errMessageContains, buf.String()) + } else { + 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 == kudoinit.DefaultManagerName { + kudoControllerFound = true + } + } + assert.True(t, kudoControllerFound, fmt.Sprintf("No kudo-controller-manager statefulset found in namespace %s", namespace)) + } + }) } - assert.True(t, kudoControllerFound, fmt.Sprintf("No kudo-controller-manager statefulset found in namespace %s", namespace)) } func TestNoErrorOnReInit(t *testing.T) { diff --git a/pkg/kudoctl/kudoinit/prereq/webhook.go b/pkg/kudoctl/kudoinit/prereq/webhook.go index 813488e3f..eb02a2d63 100644 --- a/pkg/kudoctl/kudoinit/prereq/webhook.go +++ b/pkg/kudoctl/kudoinit/prereq/webhook.go @@ -76,7 +76,7 @@ func (k KudoWebHook) installWithCertManager(client *kube.Client) error { } func (k KudoWebHook) installWithSelfSignedCA(client *kube.Client) error { - iaw, s, err := k.runtimeObjsWithSelfSignedCA() + iaw, s, err := k.resourcesWithSelfSignedCA() if err != nil { return nil } @@ -108,7 +108,7 @@ func (k KudoWebHook) Resources() []runtime.Object { } if k.opts.SelfSignedWebhookCA { - iaw, s, err := k.runtimeObjsWithSelfSignedCA() + iaw, s, err := k.resourcesWithSelfSignedCA() if err != nil { panic(err) } @@ -116,10 +116,10 @@ func (k KudoWebHook) Resources() []runtime.Object { } - return k.runtimeObjsWithCertManager() + return k.resourcesWithCertManager() } -func (k KudoWebHook) runtimeObjsWithCertManager() []runtime.Object { +func (k KudoWebHook) resourcesWithCertManager() []runtime.Object { av := InstanceAdmissionWebhook(k.opts.Namespace) objs := []runtime.Object{&av} objs = append(objs, &k.issuer) @@ -127,7 +127,7 @@ func (k KudoWebHook) runtimeObjsWithCertManager() []runtime.Object { return objs } -func (k KudoWebHook) runtimeObjsWithSelfSignedCA() (*admissionv1beta1.MutatingWebhookConfiguration, *corev1.Secret, error) { +func (k KudoWebHook) resourcesWithSelfSignedCA() (*admissionv1beta1.MutatingWebhookConfiguration, *corev1.Secret, error) { tinyCA, err := NewTinyCA(kudoinit.DefaultServiceName, k.opts.Namespace) if err != nil { return nil, nil, fmt.Errorf("unable to set up webhook CA: %v", err)