From af80059b45804a3c9eb51b98f2bb8eded091d677 Mon Sep 17 00:00:00 2001 From: ryane Date: Wed, 21 Feb 2018 16:27:50 -0500 Subject: [PATCH] fix(helm): add service, secret manifests in `init -o` When `helm init -o yaml|json` is run, the service and optional secret manifests will be included in the output. The manifests are included in a Kubernetes [List](https://godoc.org/k8s.io/apimachinery/pkg/apis/meta/v1#List) object. This also includes some refactoring and consolidates how manifests are output in `--dry-run` and removes some unnecessary marshaling between Kubernetes objects and strings. Fixes #3145 --- cmd/helm/init.go | 65 +++------------------- cmd/helm/init_test.go | 16 +++--- cmd/helm/installer/install.go | 89 ++++++++++++++++++++---------- cmd/helm/installer/install_test.go | 84 +++++++++++----------------- 4 files changed, 109 insertions(+), 145 deletions(-) diff --git a/cmd/helm/init.go b/cmd/helm/init.go index c8753874f..91441ab53 100644 --- a/cmd/helm/init.go +++ b/cmd/helm/init.go @@ -176,58 +176,29 @@ func (i *initCmd) run() error { i.opts.ServiceAccount = i.serviceAccount i.opts.MaxHistory = i.maxHistory - writeYAMLManifest := func(apiVersion, kind, body string, first, last bool) error { - w := i.out - if !first { - // YAML starting document boundary marker - if _, err := fmt.Fprintln(w, "---"); err != nil { - return err - } - } - if _, err := fmt.Fprintln(w, "apiVersion:", apiVersion); err != nil { - return err - } - if _, err := fmt.Fprintln(w, "kind:", kind); err != nil { - return err - } - if _, err := fmt.Fprint(w, body); err != nil { - return err - } - if !last { - return nil - } - // YAML ending document boundary marker - _, err := fmt.Fprintln(w, "...") - return err - } if len(i.opts.Output) > 0 { - var body string + var body []byte var err error - const tm = `{"apiVersion":"extensions/v1beta1","kind":"Deployment",` - if body, err = installer.DeploymentManifest(&i.opts); err != nil { + if body, err = installer.TillerManifests(&i.opts); err != nil { return err } switch i.opts.Output.String() { case "json": var out bytes.Buffer - jsonb, err := yaml.ToJSON([]byte(body)) + jsonb, err := yaml.ToJSON(body) if err != nil { return err } - buf := bytes.NewBuffer(make([]byte, 0, len(tm)+len(jsonb)-1)) - buf.WriteString(tm) - // Drop the opening object delimiter ('{'). - buf.Write(jsonb[1:]) + buf := bytes.NewBuffer(jsonb) if err := json.Indent(&out, buf.Bytes(), "", " "); err != nil { return err } if _, err = i.out.Write(out.Bytes()); err != nil { return err } - return nil case "yaml": - if err := writeYAMLManifest("extensions/v1beta1", "Deployment", body, true, false); err != nil { + if _, err = i.out.Write(body); err != nil { return err } return nil @@ -236,35 +207,17 @@ func (i *initCmd) run() error { } } if settings.Debug { - - var body string + var body []byte var err error - // write Deployment manifest - if body, err = installer.DeploymentManifest(&i.opts); err != nil { - return err - } - if err := writeYAMLManifest("extensions/v1beta1", "Deployment", body, true, false); err != nil { + // write Tiller manifests + if body, err = installer.TillerManifests(&i.opts); err != nil { return err } - // write Service manifest - if body, err = installer.ServiceManifest(i.namespace); err != nil { - return err - } - if err := writeYAMLManifest("v1", "Service", body, false, !i.opts.EnableTLS); err != nil { + if _, err = i.out.Write(body); err != nil { return err } - - // write Secret manifest - if i.opts.EnableTLS { - if body, err = installer.SecretManifest(&i.opts); err != nil { - return err - } - if err := writeYAMLManifest("v1", "Secret", body, false, true); err != nil { - return err - } - } } if i.dryRun { diff --git a/cmd/helm/init_test.go b/cmd/helm/init_test.go index 4513315b7..269b0b09a 100644 --- a/cmd/helm/init_test.go +++ b/cmd/helm/init_test.go @@ -167,15 +167,13 @@ func TestInitCmd_dryRun(t *testing.T) { t.Errorf("expected no server calls, got %d", got) } - docs := bytes.Split(buf.Bytes(), []byte("\n---")) - if got, want := len(docs), 2; got != want { - t.Fatalf("Expected document count of %d, got %d", want, got) - } - for _, doc := range docs { - var y map[string]interface{} - if err := yaml.Unmarshal(doc, &y); err != nil { - t.Errorf("Expected parseable YAML, got %q\n\t%s", doc, err) - } + list := &metav1.List{} + if err := yaml.Unmarshal(buf.Bytes(), &list); err != nil { + t.Errorf("Expected parseable List, got %q\n\t%s", buf.String(), err) + } + + if got, want := len(list.Items), 2; got != want { + t.Fatalf("Expected resource count of %d, got %d", want, got) } } diff --git a/cmd/helm/installer/install.go b/cmd/helm/installer/install.go index 230c7b39b..720a18522 100644 --- a/cmd/helm/installer/install.go +++ b/cmd/helm/installer/install.go @@ -27,7 +27,9 @@ import ( "k8s.io/api/core/v1" "k8s.io/api/extensions/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" + meta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes" corev1 "k8s.io/client-go/kubernetes/typed/core/v1" @@ -104,7 +106,7 @@ func semverCompare(image string) int { // createDeployment creates the Tiller Deployment resource. func createDeployment(client extensionsclient.DeploymentsGetter, opts *Options) error { - obj, err := deployment(opts) + obj, err := generateDeployment(opts) if err != nil { return err } @@ -113,40 +115,66 @@ func createDeployment(client extensionsclient.DeploymentsGetter, opts *Options) } -// deployment gets the deployment object that installs Tiller. -func deployment(opts *Options) (*v1beta1.Deployment, error) { - return generateDeployment(opts) +// Deployment gets the deployment object that installs Tiller. +func Deployment(opts *Options) (*v1beta1.Deployment, error) { + dep, err := generateDeployment(opts) + if err != nil { + return nil, err + } + dep.TypeMeta = metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: "extensions/v1beta1", + } + return dep, nil } // createService creates the Tiller service resource func createService(client corev1.ServicesGetter, namespace string) error { - obj := service(namespace) + obj := generateService(namespace) _, err := client.Services(obj.Namespace).Create(obj) return err } -// service gets the service object that installs Tiller. -func service(namespace string) *v1.Service { - return generateService(namespace) +// Service gets the service object that installs Tiller. +func Service(namespace string) *v1.Service { + svc := generateService(namespace) + svc.TypeMeta = metav1.TypeMeta{ + Kind: "Service", + APIVersion: "v1", + } + return svc } -// DeploymentManifest gets the manifest (as a string) that describes the Tiller Deployment -// resource. -func DeploymentManifest(opts *Options) (string, error) { - obj, err := deployment(opts) +// TillerManifests gets the Deployment, Service, and Secret (if tls-enabled) manifests +func TillerManifests(opts *Options) ([]byte, error) { + dep, err := Deployment(opts) if err != nil { - return "", err + return []byte{}, err } - buf, err := yaml.Marshal(obj) - return string(buf), err -} -// ServiceManifest gets the manifest (as a string) that describes the Tiller Service -// resource. -func ServiceManifest(namespace string) (string, error) { - obj := service(namespace) - buf, err := yaml.Marshal(obj) - return string(buf), err + svc := Service(opts.Namespace) + + objs := []runtime.Object{dep, svc} + + if opts.EnableTLS { + secret, err := Secret(opts) + if err != nil { + return []byte{}, err + } + objs = append(objs, secret) + } + + list := &metav1.List{ + TypeMeta: metav1.TypeMeta{ + Kind: "List", + APIVersion: "v1", + }, + } + if err := meta.SetList(list, objs); err != nil { + return []byte{}, err + } + buf, err := yaml.Marshal(list) + return buf, err } func generateLabels(labels map[string]string) map[string]string { @@ -320,14 +348,19 @@ func generateService(namespace string) *v1.Service { return s } -// SecretManifest gets the manifest (as a string) that describes the Tiller Secret resource. -func SecretManifest(opts *Options) (string, error) { - o, err := generateSecret(opts) +// Secret gets the Tiller secret resource. +func Secret(opts *Options) (*v1.Secret, error) { + secret, err := generateSecret(opts) if err != nil { - return "", err + return nil, err + } + + secret.TypeMeta = metav1.TypeMeta{ + Kind: "Secret", + APIVersion: "v1", } - buf, err := yaml.Marshal(o) - return string(buf), err + + return secret, nil } // createSecret creates the Tiller secret resource. diff --git a/cmd/helm/installer/install_test.go b/cmd/helm/installer/install_test.go index eaea05870..3a760a290 100644 --- a/cmd/helm/installer/install_test.go +++ b/cmd/helm/installer/install_test.go @@ -34,7 +34,7 @@ import ( "k8s.io/helm/pkg/version" ) -func TestDeploymentManifest(t *testing.T) { +func TestDeployment(t *testing.T) { tests := []struct { name string image string @@ -48,14 +48,10 @@ func TestDeploymentManifest(t *testing.T) { } for _, tt := range tests { - o, err := DeploymentManifest(&Options{Namespace: v1.NamespaceDefault, ImageSpec: tt.image, UseCanary: tt.canary}) + dep, err := Deployment(&Options{Namespace: v1.NamespaceDefault, ImageSpec: tt.image, UseCanary: tt.canary}) if err != nil { t.Fatalf("%s: error %q", tt.name, err) } - var dep v1beta1.Deployment - if err := yaml.Unmarshal([]byte(o), &dep); err != nil { - t.Fatalf("%s: error %q", tt.name, err) - } if got := dep.Spec.Template.Spec.Containers[0].Image; got != tt.expect { t.Errorf("%s: expected image %q, got %q", tt.name, tt.expect, got) @@ -71,7 +67,7 @@ func TestDeploymentManifest(t *testing.T) { } } -func TestDeploymentManifestForServiceAccount(t *testing.T) { +func TestDeploymentForServiceAccount(t *testing.T) { tests := []struct { name string image string @@ -84,22 +80,18 @@ func TestDeploymentManifestForServiceAccount(t *testing.T) { {"withoutSA", "", false, "gcr.io/kubernetes-helm/tiller:latest", "IfNotPresent", ""}, } for _, tt := range tests { - o, err := DeploymentManifest(&Options{Namespace: v1.NamespaceDefault, ImageSpec: tt.image, UseCanary: tt.canary, ServiceAccount: tt.serviceAccount}) + d, err := Deployment(&Options{Namespace: v1.NamespaceDefault, ImageSpec: tt.image, UseCanary: tt.canary, ServiceAccount: tt.serviceAccount}) if err != nil { t.Fatalf("%s: error %q", tt.name, err) } - var d v1beta1.Deployment - if err := yaml.Unmarshal([]byte(o), &d); err != nil { - t.Fatalf("%s: error %q", tt.name, err) - } if got := d.Spec.Template.Spec.ServiceAccountName; got != tt.serviceAccount { t.Errorf("%s: expected service account value %q, got %q", tt.name, tt.serviceAccount, got) } } } -func TestDeploymentManifest_WithTLS(t *testing.T) { +func TestDeployment_WithTLS(t *testing.T) { tests := []struct { opts Options name string @@ -126,15 +118,11 @@ func TestDeploymentManifest_WithTLS(t *testing.T) { }, } for _, tt := range tests { - o, err := DeploymentManifest(&tt.opts) + d, err := Deployment(&tt.opts) if err != nil { t.Fatalf("%s: error %q", tt.name, err) } - var d v1beta1.Deployment - if err := yaml.Unmarshal([]byte(o), &d); err != nil { - t.Fatalf("%s: error %q", tt.name, err) - } // verify environment variable in deployment reflect the use of tls being enabled. if got := d.Spec.Template.Spec.Containers[0].Env[2].Value; got != tt.verify { t.Errorf("%s: expected tls verify env value %q, got %q", tt.name, tt.verify, got) @@ -146,14 +134,7 @@ func TestDeploymentManifest_WithTLS(t *testing.T) { } func TestServiceManifest(t *testing.T) { - o, err := ServiceManifest(v1.NamespaceDefault) - if err != nil { - t.Fatalf("error %q", err) - } - var svc v1.Service - if err := yaml.Unmarshal([]byte(o), &svc); err != nil { - t.Fatalf("error %q", err) - } + svc := Service(v1.NamespaceDefault) if got := svc.ObjectMeta.Namespace; got != v1.NamespaceDefault { t.Errorf("expected namespace %s, got %s", v1.NamespaceDefault, got) @@ -161,7 +142,7 @@ func TestServiceManifest(t *testing.T) { } func TestSecretManifest(t *testing.T) { - o, err := SecretManifest(&Options{ + obj, err := Secret(&Options{ VerifyTLS: true, EnableTLS: true, Namespace: v1.NamespaceDefault, @@ -174,11 +155,6 @@ func TestSecretManifest(t *testing.T) { t.Fatalf("error %q", err) } - var obj v1.Secret - if err := yaml.Unmarshal([]byte(o), &obj); err != nil { - t.Fatalf("error %q", err) - } - if got := obj.ObjectMeta.Namespace; got != v1.NamespaceDefault { t.Errorf("expected namespace %s, got %s", v1.NamespaceDefault, got) } @@ -335,13 +311,13 @@ func TestInstall_canary(t *testing.T) { func TestUpgrade(t *testing.T) { image := "gcr.io/kubernetes-helm/tiller:v2.0.0" serviceAccount := "newServiceAccount" - existingDeployment, _ := deployment(&Options{ + existingDeployment, _ := generateDeployment(&Options{ Namespace: v1.NamespaceDefault, ImageSpec: "imageToReplace:v1.0.0", ServiceAccount: "serviceAccountToReplace", UseCanary: false, }) - existingService := service(v1.NamespaceDefault) + existingService := generateService(v1.NamespaceDefault) fc := &fake.Clientset{} fc.AddReactor("get", "deployments", func(action testcore.Action) (bool, runtime.Object, error) { @@ -376,7 +352,7 @@ func TestUpgrade(t *testing.T) { func TestUpgrade_serviceNotFound(t *testing.T) { image := "gcr.io/kubernetes-helm/tiller:v2.0.0" - existingDeployment, _ := deployment(&Options{ + existingDeployment, _ := generateDeployment(&Options{ Namespace: v1.NamespaceDefault, ImageSpec: "imageToReplace", UseCanary: false, @@ -419,13 +395,13 @@ func TestUpgrade_serviceNotFound(t *testing.T) { func TestUgrade_newerVersion(t *testing.T) { image := "gcr.io/kubernetes-helm/tiller:v2.0.0" serviceAccount := "newServiceAccount" - existingDeployment, _ := deployment(&Options{ + existingDeployment, _ := generateDeployment(&Options{ Namespace: v1.NamespaceDefault, ImageSpec: "imageToReplace:v100.5.0", ServiceAccount: "serviceAccountToReplace", UseCanary: false, }) - existingService := service(v1.NamespaceDefault) + existingService := generateService(v1.NamespaceDefault) fc := &fake.Clientset{} fc.AddReactor("get", "deployments", func(action testcore.Action) (bool, runtime.Object, error) { @@ -479,13 +455,13 @@ func TestUgrade_newerVersion(t *testing.T) { func TestUpgrade_identical(t *testing.T) { image := "gcr.io/kubernetes-helm/tiller:v2.0.0" serviceAccount := "newServiceAccount" - existingDeployment, _ := deployment(&Options{ + existingDeployment, _ := generateDeployment(&Options{ Namespace: v1.NamespaceDefault, ImageSpec: "imageToReplace:v2.0.0", ServiceAccount: "serviceAccountToReplace", UseCanary: false, }) - existingService := service(v1.NamespaceDefault) + existingService := generateService(v1.NamespaceDefault) fc := &fake.Clientset{} fc.AddReactor("get", "deployments", func(action testcore.Action) (bool, runtime.Object, error) { @@ -520,13 +496,13 @@ func TestUpgrade_identical(t *testing.T) { func TestUpgrade_canaryClient(t *testing.T) { image := "gcr.io/kubernetes-helm/tiller:canary" serviceAccount := "newServiceAccount" - existingDeployment, _ := deployment(&Options{ + existingDeployment, _ := generateDeployment(&Options{ Namespace: v1.NamespaceDefault, ImageSpec: "imageToReplace:v1.0.0", ServiceAccount: "serviceAccountToReplace", UseCanary: false, }) - existingService := service(v1.NamespaceDefault) + existingService := generateService(v1.NamespaceDefault) fc := &fake.Clientset{} fc.AddReactor("get", "deployments", func(action testcore.Action) (bool, runtime.Object, error) { @@ -561,13 +537,13 @@ func TestUpgrade_canaryClient(t *testing.T) { func TestUpgrade_canaryServer(t *testing.T) { image := "gcr.io/kubernetes-helm/tiller:v2.0.0" serviceAccount := "newServiceAccount" - existingDeployment, _ := deployment(&Options{ + existingDeployment, _ := generateDeployment(&Options{ Namespace: v1.NamespaceDefault, ImageSpec: "imageToReplace:canary", ServiceAccount: "serviceAccountToReplace", UseCanary: false, }) - existingService := service(v1.NamespaceDefault) + existingService := generateService(v1.NamespaceDefault) fc := &fake.Clientset{} fc.AddReactor("get", "deployments", func(action testcore.Action) (bool, runtime.Object, error) { @@ -607,7 +583,8 @@ func tlsTestFile(t *testing.T, path string) string { } return path } -func TestDeploymentManifest_WithNodeSelectors(t *testing.T) { + +func TestDeployment_WithNodeSelectors(t *testing.T) { tests := []struct { opts Options name string @@ -631,15 +608,11 @@ func TestDeploymentManifest_WithNodeSelectors(t *testing.T) { }, } for _, tt := range tests { - o, err := DeploymentManifest(&tt.opts) + d, err := Deployment(&tt.opts) if err != nil { t.Fatalf("%s: error %q", tt.name, err) } - var d v1beta1.Deployment - if err := yaml.Unmarshal([]byte(o), &d); err != nil { - t.Fatalf("%s: error %q", tt.name, err) - } // Verify that environment variables in Deployment reflect the use of TLS being enabled. got := d.Spec.Template.Spec.NodeSelector for k, v := range tt.expect { @@ -649,7 +622,8 @@ func TestDeploymentManifest_WithNodeSelectors(t *testing.T) { } } } -func TestDeploymentManifest_WithSetValues(t *testing.T) { + +func TestDeployment_WithSetValues(t *testing.T) { tests := []struct { opts Options name string @@ -676,11 +650,17 @@ func TestDeploymentManifest_WithSetValues(t *testing.T) { }, } for _, tt := range tests { - o, err := DeploymentManifest(&tt.opts) + d, err := Deployment(&tt.opts) if err != nil { t.Fatalf("%s: error %q", tt.name, err) } - values, err := chartutil.ReadValues([]byte(o)) + + o, err := yaml.Marshal(d) + if err != nil { + t.Errorf("Error marshaling Deployment: %s", err) + } + + values, err := chartutil.ReadValues(o) if err != nil { t.Errorf("Error converting Deployment manifest to Values: %s", err) }