From c4383da11ed5e6efa034c1b04da8848546a2f390 Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Tue, 26 Mar 2019 00:26:46 +0000 Subject: [PATCH 01/12] cli: Introduce an upgrade command The `install` command errors when the deploy target contains an existing Linkerd deployment. The `upgrade` command is introduced to reinstall or reconfigure the Linkerd control plane. Upgrade works as follows: 1. The controller config is fetched from the Kubernetes API. The Public API is not used, because we need to be able to reinstall the control plane when the Public API is not available; and we are not concerned about RBAC restrictions preventing the installer from reading the config (as we are for inject). 2. The install configuration is read, particularly the flags used during the last install/upgrade. If these flags were not set again during the upgrade, the previous values are used as if they were passed this time. The configuration is updated from the combination of these values, including the install configuration itself. Note that some flags, including the linkerd-version, are omitted since they are stored elsewhere in the configurations and don't make sense to track as overrides.. 3. The issuer secrets are read from the Kubernetes API so that they can be re-used. There is currently no way to reconfigure issuer certificates. We will need to create _another_ workflow for updating these credentials. 4. The install rendering is invoked with values and config fetched from the cluster, synthesized with the new configuration. --- cli/cmd/inject.go | 2 +- cli/cmd/inject_test.go | 2 +- cli/cmd/install.go | 92 ++++++++++--------- cli/cmd/install_test.go | 10 +-- cli/cmd/root.go | 1 + cli/cmd/upgrade.go | 193 ++++++++++++++++++++++++++++++++++++++++ pkg/config/config.go | 26 +++++- pkg/k8s/labels.go | 4 + 8 files changed, 278 insertions(+), 52 deletions(-) create mode 100644 cli/cmd/upgrade.go diff --git a/cli/cmd/inject.go b/cli/cmd/inject.go index 5bdcf0e471a81..51c7e35764453 100644 --- a/cli/cmd/inject.go +++ b/cli/cmd/inject.go @@ -295,7 +295,7 @@ func (options *injectOptions) fetchConfigsOrDefault() (*config.All, error) { // overrideConfigs uses command-line overrides to update the provided configs. // the overrideAnnotations map keeps track of which configs are overridden, by // storing the corresponding annotations and values. -func (options *injectOptions) overrideConfigs(configs *config.All, overrideAnnotations map[string]string) { +func (options *proxyConfigOptions) overrideConfigs(configs *config.All, overrideAnnotations map[string]string) { if options.linkerdVersion != "" { configs.Global.Version = options.linkerdVersion } diff --git a/cli/cmd/inject_test.go b/cli/cmd/inject_test.go index e721e6525cbcc..f2e86386ef8e1 100644 --- a/cli/cmd/inject_test.go +++ b/cli/cmd/inject_test.go @@ -54,7 +54,7 @@ func testUninjectAndInject(t *testing.T, tc testCase) { } func testInstallConfig() *pb.All { - _, c, err := testInstallOptions().validateAndBuild() + _, c, err := testInstallOptions().validateAndBuild(nil) if err != nil { log.Fatalf("test install options must be valid: %s", err) } diff --git a/cli/cmd/install.go b/cli/cmd/install.go index 0e49523cafefc..dcf55db5c67e7 100644 --- a/cli/cmd/install.go +++ b/cli/cmd/install.go @@ -208,12 +208,15 @@ func newCmdInstall() *cobra.Command { Short: "Output Kubernetes configs to install Linkerd", Long: "Output Kubernetes configs to install Linkerd.", RunE: func(cmd *cobra.Command, args []string) error { - options.recordFlags(flags) + if !options.ignoreCluster { + exitIfClusterExists() + } - values, configs, err := options.validateAndBuild() + values, configs, err := options.validateAndBuild(flags) if err != nil { return err } + return values.render(os.Stdout, configs) }, } @@ -226,6 +229,28 @@ func newCmdInstall() *cobra.Command { return cmd } +func (options *installOptions) validateAndBuild(flags *pflag.FlagSet) (*installValues, *pb.All, error) { + if err := options.validate(); err != nil { + return nil, nil, err + } + options.recordFlags(flags) + + identityValues, err := options.identityOptions.validateAndBuild() + if err != nil { + return nil, nil, err + } + + configs := options.configs(identityValues.toIdentityContext()) + + values, err := options.buildValuesWithoutIdentity(configs) + if err != nil { + return nil, nil, err + } + values.Identity = identityValues + + return values, configs, nil +} + func (options *installOptions) flagSet(e pflag.ErrorHandling) *pflag.FlagSet { flags := pflag.NewFlagSet("install", e) @@ -305,6 +330,7 @@ func (options *installOptions) recordFlags(flags *pflag.FlagSet) { case "ignore-cluster", "linkerd-version": // These flags don't make sense to record. default: + fmt.Printf("flags %s %v\n", f.Name, f.Changed) options.recordedFlags = append(options.recordedFlags, &pb.Install_Flag{ Name: f.Name, Value: f.Value.String(), @@ -331,26 +357,6 @@ func (options *installOptions) validate() error { return errors.New("--proxy-log-level must not be empty") } - if !options.ignoreCluster { - exists, err := linkerdConfigAlreadyExistsInCluster() - if err != nil { - fmt.Fprintln(os.Stderr, "Unable to connect to a Kubernetes cluster to check for configuration. If this expected, use the --ignore-cluster flag.") - os.Exit(1) - } - if exists { - fmt.Fprintln(os.Stderr, "You are already running a control plane. If you would like to ignore its configuration, use the --ignore-cluster flag.") - os.Exit(1) - } - } - - return nil -} - -func (options *installOptions) validateAndBuild() (*installValues, *pb.All, error) { - if err := options.validate(); err != nil { - return nil, nil, err - } - if options.highAvailability { if options.controllerReplicas == defaultControllerReplicas { options.controllerReplicas = defaultHAControllerReplicas @@ -366,16 +372,14 @@ func (options *installOptions) validateAndBuild() (*installValues, *pb.All, erro } options.identityOptions.replicas = options.controllerReplicas - identityValues, err := options.identityOptions.validateAndBuild() - if err != nil { - return nil, nil, err - } - configs := options.configs(identityValues.toIdentityContext()) + return nil +} +func (options *installOptions) buildValuesWithoutIdentity(configs *pb.All) (*installValues, error) { globalJSON, proxyJSON, installJSON, err := config.ToJSON(configs) if err != nil { - return nil, nil, err + return nil, err } values := &installValues{ @@ -410,8 +414,6 @@ func (options *installOptions) validateAndBuild() (*installValues, *pb.All, erro Proxy: proxyJSON, Install: installJSON, }, - - Identity: identityValues, } if options.highAvailability { @@ -438,7 +440,7 @@ func (options *installOptions) validateAndBuild() (*installValues, *pb.All, erro } } - return values, configs, nil + return values, nil } func toPromLogLevel(level string) string { @@ -622,34 +624,40 @@ func (options *installOptions) proxyConfig() *pb.Proxy { } } -// linkerdConfigAlreadyExistsInCluster checks the kubernetes API to determine -// whether a config exists. +// exitIfClusterExists checks the kubernetes API to determine +// whether a config exists and exits if it does exist or if an error is +// encountered. // // This bypasses the public API so that public API errors cannot cause us to // misdiagnose a controller error to indicate that no control plane exists. -// -// If we cannot determine whether the configuration exists, an error is returned. -func linkerdConfigAlreadyExistsInCluster() (bool, error) { +func exitIfClusterExists() { api, err := k8s.NewAPI(kubeconfigPath, kubeContext) if err != nil { - return false, err + fmt.Fprintln(os.Stderr, "Unable to build a Kubernetes client to check for configuration. If this expected, use the --ignore-cluster flag.") + fmt.Fprintf(os.Stderr, "Error: %s\n", err) + os.Exit(1) } k, err := kubernetes.NewForConfig(api.Config) if err != nil { - return false, err + fmt.Fprintln(os.Stderr, "Unable to build a Kubernetes client to check for configuration. If this expected, use the --ignore-cluster flag.") + fmt.Fprintf(os.Stderr, "Error: %s\n", err) + os.Exit(1) } c := k.CoreV1().ConfigMaps(controlPlaneNamespace) if _, err = c.Get(k8s.ConfigConfigMapName, metav1.GetOptions{}); err != nil { if kerrors.IsNotFound(err) { - return false, nil + return } - return false, err + fmt.Fprintln(os.Stderr, "Unable to build a Kubernetes client to check for configuration. If this expected, use the --ignore-cluster flag.") + fmt.Fprintf(os.Stderr, "Error: %s\n", err) + os.Exit(1) } - return true, nil + fmt.Fprintln(os.Stderr, "You are already running a control plane. If you would like to ignore its configuration, use the --ignore-cluster flag.") + os.Exit(1) } func (idopts *installIdentityOptions) validate() error { @@ -657,7 +665,7 @@ func (idopts *installIdentityOptions) validate() error { return nil } - if idopts.trustDomain == "" { + if idopts.trustDomain != "" { if errs := validation.IsDNS1123Subdomain(idopts.trustDomain); len(errs) > 0 { return fmt.Errorf("invalid trust domain '%s': %s", idopts.trustDomain, errs[0]) } diff --git a/cli/cmd/install_test.go b/cli/cmd/install_test.go index 4599bf488f7b2..d9297651ed5e0 100644 --- a/cli/cmd/install_test.go +++ b/cli/cmd/install_test.go @@ -11,7 +11,7 @@ import ( func TestRender(t *testing.T) { defaultOptions := testInstallOptions() - defaultValues, defaultConfig, err := defaultOptions.validateAndBuild() + defaultValues, defaultConfig, err := defaultOptions.validateAndBuild(nil) if err != nil { t.Fatalf("Unexpected error validating options: %v", err) } @@ -53,7 +53,7 @@ func TestRender(t *testing.T) { haOptions := testInstallOptions() haOptions.recordedFlags = []*config.Install_Flag{{Name: "ha", Value: "true"}} haOptions.highAvailability = true - haValues, haConfig, _ := haOptions.validateAndBuild() + haValues, haConfig, _ := haOptions.validateAndBuild(nil) haWithOverridesOptions := testInstallOptions() haWithOverridesOptions.recordedFlags = []*config.Install_Flag{ @@ -66,12 +66,12 @@ func TestRender(t *testing.T) { haWithOverridesOptions.controllerReplicas = 2 haWithOverridesOptions.proxyCPURequest = "400m" haWithOverridesOptions.proxyMemoryRequest = "300Mi" - haWithOverridesValues, haWithOverridesConfig, _ := haWithOverridesOptions.validateAndBuild() + haWithOverridesValues, haWithOverridesConfig, _ := haWithOverridesOptions.validateAndBuild(nil) noInitContainerOptions := testInstallOptions() noInitContainerOptions.recordedFlags = []*config.Install_Flag{{Name: "linkerd-cni-enabled", Value: "true"}} noInitContainerOptions.noInitContainer = true - noInitContainerValues, noInitContainerConfig, _ := noInitContainerOptions.validateAndBuild() + noInitContainerValues, noInitContainerConfig, _ := noInitContainerOptions.validateAndBuild(nil) noInitContainerWithProxyAutoInjectOptions := testInstallOptions() noInitContainerWithProxyAutoInjectOptions.recordedFlags = []*config.Install_Flag{ @@ -80,7 +80,7 @@ func TestRender(t *testing.T) { } noInitContainerWithProxyAutoInjectOptions.noInitContainer = true noInitContainerWithProxyAutoInjectOptions.proxyAutoInject = true - noInitContainerWithProxyAutoInjectValues, noInitContainerWithProxyAutoInjectConfig, _ := noInitContainerWithProxyAutoInjectOptions.validateAndBuild() + noInitContainerWithProxyAutoInjectValues, noInitContainerWithProxyAutoInjectConfig, _ := noInitContainerWithProxyAutoInjectOptions.validateAndBuild(nil) testCases := []struct { values *installValues diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 6e47ed9960592..9731dc4c8f2a0 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -112,6 +112,7 @@ func init() { RootCmd.AddCommand(newCmdTap()) RootCmd.AddCommand(newCmdTop()) RootCmd.AddCommand(newCmdUninject()) + RootCmd.AddCommand(newCmdUpgrade()) RootCmd.AddCommand(newCmdVersion()) } diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go new file mode 100644 index 0000000000000..2082800fa1978 --- /dev/null +++ b/cli/cmd/upgrade.go @@ -0,0 +1,193 @@ +package cmd + +import ( + "errors" + "fmt" + "os" + "time" + + pb "github.com/linkerd/linkerd2/controller/gen/config" + "github.com/linkerd/linkerd2/pkg/config" + "github.com/linkerd/linkerd2/pkg/k8s" + "github.com/linkerd/linkerd2/pkg/tls" + "github.com/spf13/cobra" + "github.com/spf13/pflag" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" +) + +type ( + upgradeOptions struct{ *installOptions } +) + +func newUpgradeOptionsWithDefaults() *upgradeOptions { + return &upgradeOptions{newInstallOptionsWithDefaults()} +} + +func newCmdUpgrade() *cobra.Command { + options := newUpgradeOptionsWithDefaults() + flags := options.flagSet(pflag.ExitOnError) + + cmd := &cobra.Command{ + Use: "upgrade [flags]", + Short: "Output Kubernetes configs to upgrade an existing Linkerd control plane", + Long: "Output Kubernetes configs to upgrade an existing Linkerd control plane.", + RunE: func(cmd *cobra.Command, args []string) error { + // We need a Kubernetes client to fetch configs and issuer secrets. + k, err := options.newK8s() + if err != nil { + return fmt.Errorf("failed to create a kubernetes client: %s", err) + } + + // We fetch the configs directly from kubernetes because we need to be able + // to upgrade/reinstall the control plane when the API is not available; and + // this also serves as a passive check that we have privileges to access this + // control plane. + configs, err := fetchConfigs(k) + if err != nil { + return fmt.Errorf("could not fetch configs from kubernetes: %s", err) + } + + // We recorded flags during a prior install. If we haven't overridden the + // flag on this upgrade, reset that prior value as if it were specified now. + setOptionsFromInstall(flags, configs.GetInstall()) + + if err = options.validate(); err != nil { + return err + } + + // Save off the updated set of flags into the installOptions so it gets + // persisted with the upgraded config. + options.recordFlags(flags) + + // Update the configs from the synthesized options. + options.overrideConfigs(configs, map[string]string{}) + configs.GetInstall().Flags = options.recordedFlags + + values, err := options.buildValuesWithoutIdentity(configs) + if err != nil { + return fmt.Errorf("could not build install configuration: %s", err) + } + + identityValues, err := fetchIdentityValues(k, options.controllerReplicas, configs.GetGlobal().GetIdentityContext()) + if err != nil { + fmt.Fprintln(os.Stderr, "Unable to fetch the existing issuer credentials from Kubernetes.") + fmt.Fprintf(os.Stderr, "Error: %s\n", err) + os.Exit(1) + } + values.Identity = identityValues + + if err = values.render(os.Stdout, configs); err != nil { + return fmt.Errorf("could not render install configuration: %s", err) + } + + return nil + }, + } + + cmd.PersistentFlags().AddFlagSet(flags) + return cmd +} + +func setOptionsFromInstall(flags *pflag.FlagSet, install *pb.Install) { + for _, i := range install.GetFlags() { + fmt.Printf("flags: setting: %s %s\n", i.GetName(), i.GetValue()) + if f := flags.Lookup(i.GetName()); f != nil && !f.Changed { + f.Value.Set(i.GetValue()) + f.Changed = true + } + } +} + +func (options *upgradeOptions) newK8s() (*kubernetes.Clientset, error) { + if options.ignoreCluster { + return nil, errors.New("--ignore-cluster cannot be used with upgrade") + } + + api, err := k8s.NewAPI(kubeconfigPath, kubeContext) + if err != nil { + return nil, err + } + + return kubernetes.NewForConfig(api.Config) +} + +// fetchIdentityValue checks the kubernetes API to fetch an existing +// linkerd configuration. +// +// This bypasses the public API so that upgrades can proceed when the API pod is +// not available. +func fetchConfigs(k *kubernetes.Clientset) (*pb.All, error) { + configMap, err := k.CoreV1(). + ConfigMaps(controlPlaneNamespace). + Get(k8s.ConfigConfigMapName, metav1.GetOptions{}) + if err != nil { + return nil, err + } + + return config.FromConfigMap(configMap.Data) +} + +// fetchIdentityValue checks the kubernetes API to fetch an existing +// linkerd identity configuration. +// +// This bypasses the public API so that we can access secrets and validate +// permissions. +func fetchIdentityValues(k *kubernetes.Clientset, replicas uint, idctx *pb.IdentityContext) (*installIdentityValues, error) { + if idctx == nil { + return nil, nil + } + + keyPEM, crtPEM, expiry, err := fetchIssuer(k, idctx.GetTrustAnchorsPem()) + if err != nil { + return nil, err + } + + return &installIdentityValues{ + Replicas: replicas, + TrustDomain: idctx.GetTrustDomain(), + TrustAnchorsPEM: idctx.GetTrustAnchorsPem(), + Issuer: &issuerValues{ + ClockSkewAllowance: idctx.GetClockSkewAllowance().String(), + IssuanceLifetime: idctx.GetIssuanceLifetime().String(), + CrtExpiryAnnotation: k8s.IdentityIssuerExpiryAnnotation, + + KeyPEM: keyPEM, + CrtPEM: crtPEM, + CrtExpiry: expiry, + }, + }, nil +} + +func fetchIssuer(k *kubernetes.Clientset, trustPEM string) (string, string, time.Time, error) { + roots, err := tls.DecodePEMCertPool(trustPEM) + if err != nil { + return "", "", time.Time{}, err + } + + secret, err := k.CoreV1(). + Secrets(controlPlaneNamespace). + Get(k8s.IdentityIssuerSecretName, metav1.GetOptions{}) + if err != nil { + return "", "", time.Time{}, err + } + + keyPEM := string(secret.Data["key.pem"]) + key, err := tls.DecodePEMKey(keyPEM) + if err != nil { + return "", "", time.Time{}, err + } + + crtPEM := string(secret.Data["crt.pem"]) + crt, err := tls.DecodePEMCrt(crtPEM) + if err != nil { + return "", "", time.Time{}, err + } + + cred := &tls.Cred{PrivateKey: key, Crt: *crt} + if err = cred.Verify(roots, ""); err != nil { + return "", "", time.Time{}, fmt.Errorf("invalid issuer credentials: %s", err) + } + + return keyPEM, crtPEM, crt.Certificate.NotAfter, nil +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 26a65da97f13d..118cddb42dd59 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -14,18 +14,18 @@ import ( // Global returns the Global protobuf config from the linkerd-config ConfigMap func Global(filepath string) (*pb.Global, error) { config := &pb.Global{} - err := unmarshalConfig(filepath, config) + err := unmarshalFile(filepath, config) return config, err } // Proxy returns the Proxy protobuf config from the linkerd-config ConfigMap func Proxy(filepath string) (*pb.Proxy, error) { config := &pb.Proxy{} - err := unmarshalConfig(filepath, config) + err := unmarshalFile(filepath, config) return config, err } -func unmarshalConfig(filepath string, msg proto.Message) error { +func unmarshalFile(filepath string, msg proto.Message) error { configJSON, err := ioutil.ReadFile(filepath) if err != nil { return fmt.Errorf("failed to read config file: %s", err) @@ -46,6 +46,26 @@ func unmarshal(json string, msg proto.Message) error { return u.Unmarshal(strings.NewReader(json), msg) } +// FromConfigMap builds a configuration by reading a map with the keys "global" +// and "proxy", each containing JSON values. +func FromConfigMap(configMap map[string]string) (*pb.All, error) { + c := &pb.All{Global: &pb.Global{}, Proxy: &pb.Proxy{}, Install: &pb.Install{}} + + if err := unmarshal(configMap["global"], c.Global); err != nil { + return nil, fmt.Errorf("global: %s", err) + } + + if err := unmarshal(configMap["proxy"], c.Proxy); err != nil { + return nil, fmt.Errorf("proxy: %s", err) + } + + if err := unmarshal(configMap["install"], c.Install); err != nil { + return nil, fmt.Errorf("install: %s", err) + } + + return c, nil +} + // ToJSON encode the configuration to JSON, i.e. to be stored in a ConfigMap. func ToJSON(configs *pb.All) (global, proxy, install string, err error) { m := jsonpb.Marshaler{EmitDefaults: true} diff --git a/pkg/k8s/labels.go b/pkg/k8s/labels.go index 1ee92c20bea3b..86b4a1ae95210 100644 --- a/pkg/k8s/labels.go +++ b/pkg/k8s/labels.go @@ -173,6 +173,10 @@ const ( // volume mounted into each proxy to store identity credentials. IdentityEndEntityVolumeName = "linkerd-identity-end-entity" + // IdentityIssuerSecretName is the name assigned the temporary end-entity + // volume mounted into each proxy to store identity credentials. + IdentityIssuerSecretName = "linkerd-identity-issuer" + // ProxyPortName is the name of the Linkerd Proxy's proxy port. ProxyPortName = "linkerd-proxy" From 292f1daa637f3c4df44afc55aa5004b7f394dbe4 Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Fri, 29 Mar 2019 17:53:32 +0000 Subject: [PATCH 02/12] const up issuer file names --- cli/cmd/upgrade.go | 4 ++-- pkg/k8s/labels.go | 9 +++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go index 2082800fa1978..0bebeb6a1ec75 100644 --- a/cli/cmd/upgrade.go +++ b/cli/cmd/upgrade.go @@ -172,13 +172,13 @@ func fetchIssuer(k *kubernetes.Clientset, trustPEM string) (string, string, time return "", "", time.Time{}, err } - keyPEM := string(secret.Data["key.pem"]) + keyPEM := string(secret.Data[k8s.IdentityIssuerKeyName]) key, err := tls.DecodePEMKey(keyPEM) if err != nil { return "", "", time.Time{}, err } - crtPEM := string(secret.Data["crt.pem"]) + crtPEM := string(secret.Data[k8s.IdentityIssuerCrtName]) crt, err := tls.DecodePEMCrt(crtPEM) if err != nil { return "", "", time.Time{}, err diff --git a/pkg/k8s/labels.go b/pkg/k8s/labels.go index 86b4a1ae95210..0f1902ee0fa99 100644 --- a/pkg/k8s/labels.go +++ b/pkg/k8s/labels.go @@ -173,10 +173,15 @@ const ( // volume mounted into each proxy to store identity credentials. IdentityEndEntityVolumeName = "linkerd-identity-end-entity" - // IdentityIssuerSecretName is the name assigned the temporary end-entity - // volume mounted into each proxy to store identity credentials. + // IdentityIssuerSecretName is the name of the Secret that stores issuer credentials. IdentityIssuerSecretName = "linkerd-identity-issuer" + // IdentityIssuerKeyName is the issuer's private key file. + IdentityIssuerKeyName = "key.pem" + + // IdentityIssuerKeyName is the issuer's certificate file. + IdentityIssuerCrtName = "crt.pem" + // ProxyPortName is the name of the Linkerd Proxy's proxy port. ProxyPortName = "linkerd-proxy" From e813001c97ef98b13c2100982633a4b6144171c3 Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Fri, 29 Mar 2019 18:01:08 +0000 Subject: [PATCH 03/12] fixup doc --- cli/cmd/upgrade.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go index 0bebeb6a1ec75..9034943baeeaa 100644 --- a/cli/cmd/upgrade.go +++ b/cli/cmd/upgrade.go @@ -112,7 +112,7 @@ func (options *upgradeOptions) newK8s() (*kubernetes.Clientset, error) { return kubernetes.NewForConfig(api.Config) } -// fetchIdentityValue checks the kubernetes API to fetch an existing +// fetchConfigs checks the kubernetes API to fetch an existing // linkerd configuration. // // This bypasses the public API so that upgrades can proceed when the API pod is From 83ad264a454beb36c0fb67faed7dcac7a03944a2 Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Fri, 29 Mar 2019 20:09:02 +0000 Subject: [PATCH 04/12] fixup lint --- pkg/k8s/labels.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/k8s/labels.go b/pkg/k8s/labels.go index 0f1902ee0fa99..4250d9385a104 100644 --- a/pkg/k8s/labels.go +++ b/pkg/k8s/labels.go @@ -179,7 +179,7 @@ const ( // IdentityIssuerKeyName is the issuer's private key file. IdentityIssuerKeyName = "key.pem" - // IdentityIssuerKeyName is the issuer's certificate file. + // IdentityIssuerCrtName is the issuer's certificate file. IdentityIssuerCrtName = "crt.pem" // ProxyPortName is the name of the Linkerd Proxy's proxy port. From b72b057403d2c7a7d3a825b29aaaf058fd65d5d1 Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Fri, 29 Mar 2019 20:10:09 +0000 Subject: [PATCH 05/12] fixup error message --- cli/cmd/install.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/cmd/install.go b/cli/cmd/install.go index dcf55db5c67e7..ca1d9b1a810eb 100644 --- a/cli/cmd/install.go +++ b/cli/cmd/install.go @@ -656,7 +656,7 @@ func exitIfClusterExists() { os.Exit(1) } - fmt.Fprintln(os.Stderr, "You are already running a control plane. If you would like to ignore its configuration, use the --ignore-cluster flag.") + fmt.Fprintln(os.Stderr, "Linkerd has already been installed on your cluster in the linkerd namespace. Please run upgrade if you'd like to update this installation. Otherwise, use the --ignore-cluster flag.") os.Exit(1) } From f394fae29299a33dd6f5a3eb22fb863e6e7a9ee5 Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Fri, 29 Mar 2019 20:33:00 +0000 Subject: [PATCH 06/12] Only expose flags when they can be used --- cli/cmd/inject.go | 9 +++++++-- cli/cmd/install.go | 25 ++++++++++++++----------- cli/cmd/root.go | 5 ----- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/cli/cmd/inject.go b/cli/cmd/inject.go index 51c7e35764453..5fcf349233d04 100644 --- a/cli/cmd/inject.go +++ b/cli/cmd/inject.go @@ -102,11 +102,16 @@ sub-folders, or coming from stdin.`, }, } - cmd.PersistentFlags().AddFlagSet(options.proxyConfigOptions.flagSet(pflag.ExitOnError)) - cmd.PersistentFlags().BoolVar( + flags := options.proxyConfigOptions.flagSet(pflag.ExitOnError) + flags.BoolVar( &options.disableIdentity, "disable-identity", options.disableIdentity, "Disables resources from participating in TLS identity", ) + flags.BoolVar( + &options.ignoreCluster, "ignore-cluster", options.ignoreCluster, + "Ignore the current Kubernetes cluster when checking for existing cluster configuration (default false)", + ) + cmd.PersistentFlags().AddFlagSet(flags) return cmd } diff --git a/cli/cmd/install.go b/cli/cmd/install.go index ca1d9b1a810eb..a41afa0ab9d07 100644 --- a/cli/cmd/install.go +++ b/cli/cmd/install.go @@ -223,8 +223,8 @@ func newCmdInstall() *cobra.Command { cmd.PersistentFlags().AddFlagSet(flags) - // Issuer flags are currently only supported on the initial install. - cmd.PersistentFlags().AddFlagSet(options.issuerFlagSet(pflag.ExitOnError)) + // Some flags are not available during upgrade, etc. + cmd.PersistentFlags().AddFlagSet(options.installOnlyFlagSet(pflag.ExitOnError)) return cmd } @@ -251,6 +251,7 @@ func (options *installOptions) validateAndBuild(flags *pflag.FlagSet) (*installV return values, configs, nil } +// flagSet returns flags usable during install or upgrade. func (options *installOptions) flagSet(e pflag.ErrorHandling) *pflag.FlagSet { flags := pflag.NewFlagSet("install", e) @@ -284,12 +285,17 @@ func (options *installOptions) flagSet(e pflag.ErrorHandling) *pflag.FlagSet { &options.identityOptions.issuanceLifetime, "identity-issuance-lifetime", options.identityOptions.issuanceLifetime, "The amount of time for which the Identity issuer should certify identity", ) + flags.DurationVar( + &options.identityOptions.clockSkewAllowance, "identity-clock-skew-allowance", options.identityOptions.clockSkewAllowance, + "The amount of time to allow for clock skew within a Linkerd cluster", + ) return flags } -func (options *installOptions) issuerFlagSet(e pflag.ErrorHandling) *pflag.FlagSet { - flags := pflag.NewFlagSet("issuer", e) +// installOnlyFlagSet includes flags that are only accessible at install-time and not at upgrade-time. +func (options *installOptions) installOnlyFlagSet(e pflag.ErrorHandling) *pflag.FlagSet { + flags := pflag.NewFlagSet("install", e) flags.StringVar( &options.identityOptions.trustDomain, "identity-trust-domain", options.identityOptions.trustDomain, @@ -307,13 +313,10 @@ func (options *installOptions) issuerFlagSet(e pflag.ErrorHandling) *pflag.FlagS &options.identityOptions.keyPEMFile, "identity-issuer-key-file", options.identityOptions.keyPEMFile, "A path to a PEM-encoded file containing the Linkerd Identity issuer private key (generated by default)", ) - flags.DurationVar( - &options.identityOptions.clockSkewAllowance, "identity-clock-skew-allowance", options.identityOptions.clockSkewAllowance, - "The amount of time to allow for clock skew within a Linkerd cluster", - ) - flags.DurationVar( - &options.identityOptions.issuanceLifetime, "identity-issuance-lifetime", options.identityOptions.issuanceLifetime, - "The amount of time for which the Identity issuer should certify identity", + + flags.BoolVar( + &options.ignoreCluster, "ignore-cluster", options.ignoreCluster, + "Ignore the current Kubernetes cluster when checking for existing cluster configuration (default false)", ) return flags diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 9731dc4c8f2a0..d64306be38f52 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -301,10 +301,5 @@ func (options *proxyConfigOptions) flagSet(e pflag.ErrorHandling) *pflag.FlagSet flags.MarkDeprecated("proxy-memory", "use --proxy-memory-request instead") flags.MarkDeprecated("proxy-cpu", "use --proxy-cpu-request instead") - flags.BoolVar( - &options.ignoreCluster, "ignore-cluster", options.ignoreCluster, - "Ignore the current Kubernetes cluster when checking for existing cluster configuration (default false)", - ) - return flags } From 4095c7dc5132f25d149e8871fa238bc117f8b867 Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Fri, 29 Mar 2019 20:38:59 +0000 Subject: [PATCH 07/12] remove stupid printlns --- cli/cmd/install.go | 1 - cli/cmd/upgrade.go | 1 - 2 files changed, 2 deletions(-) diff --git a/cli/cmd/install.go b/cli/cmd/install.go index a41afa0ab9d07..a1539dcdee67c 100644 --- a/cli/cmd/install.go +++ b/cli/cmd/install.go @@ -333,7 +333,6 @@ func (options *installOptions) recordFlags(flags *pflag.FlagSet) { case "ignore-cluster", "linkerd-version": // These flags don't make sense to record. default: - fmt.Printf("flags %s %v\n", f.Name, f.Changed) options.recordedFlags = append(options.recordedFlags, &pb.Install_Flag{ Name: f.Name, Value: f.Value.String(), diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go index 9034943baeeaa..209a3f317af0d 100644 --- a/cli/cmd/upgrade.go +++ b/cli/cmd/upgrade.go @@ -91,7 +91,6 @@ func newCmdUpgrade() *cobra.Command { func setOptionsFromInstall(flags *pflag.FlagSet, install *pb.Install) { for _, i := range install.GetFlags() { - fmt.Printf("flags: setting: %s %s\n", i.GetName(), i.GetValue()) if f := flags.Lookup(i.GetName()); f != nil && !f.Changed { f.Value.Set(i.GetValue()) f.Changed = true From e23d85c91efae279ba6bcf92b2eb26e7cd2a6edf Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Fri, 29 Mar 2019 22:34:43 +0000 Subject: [PATCH 08/12] fixup kube client constructor --- cli/cmd/install.go | 4 ++-- cli/cmd/upgrade.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/cmd/install.go b/cli/cmd/install.go index a1539dcdee67c..67a46ea7e9996 100644 --- a/cli/cmd/install.go +++ b/cli/cmd/install.go @@ -633,14 +633,14 @@ func (options *installOptions) proxyConfig() *pb.Proxy { // This bypasses the public API so that public API errors cannot cause us to // misdiagnose a controller error to indicate that no control plane exists. func exitIfClusterExists() { - api, err := k8s.NewAPI(kubeconfigPath, kubeContext) + kubeConfig, err := k8s.GetConfig(kubeconfigPath, kubeContext) if err != nil { fmt.Fprintln(os.Stderr, "Unable to build a Kubernetes client to check for configuration. If this expected, use the --ignore-cluster flag.") fmt.Fprintf(os.Stderr, "Error: %s\n", err) os.Exit(1) } - k, err := kubernetes.NewForConfig(api.Config) + k, err := kubernetes.NewForConfig(kubeConfig) if err != nil { fmt.Fprintln(os.Stderr, "Unable to build a Kubernetes client to check for configuration. If this expected, use the --ignore-cluster flag.") fmt.Fprintf(os.Stderr, "Error: %s\n", err) diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go index 209a3f317af0d..b0ca01e1c5358 100644 --- a/cli/cmd/upgrade.go +++ b/cli/cmd/upgrade.go @@ -103,12 +103,12 @@ func (options *upgradeOptions) newK8s() (*kubernetes.Clientset, error) { return nil, errors.New("--ignore-cluster cannot be used with upgrade") } - api, err := k8s.NewAPI(kubeconfigPath, kubeContext) + c, err := k8s.GetConfig(kubeconfigPath, kubeContext) if err != nil { return nil, err } - return kubernetes.NewForConfig(api.Config) + return kubernetes.NewForConfig(c) } // fetchConfigs checks the kubernetes API to fetch an existing From 85447ac3d0c9e93716c67c65ef1089cc1cf1b1e1 Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Mon, 1 Apr 2019 19:21:48 +0000 Subject: [PATCH 09/12] fixup per review --- cli/cmd/upgrade.go | 12 +++++++++--- controller/cmd/identity/main.go | 5 ++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go index b0ca01e1c5358..2d38b083fcc6e 100644 --- a/cli/cmd/upgrade.go +++ b/cli/cmd/upgrade.go @@ -1,7 +1,6 @@ package cmd import ( - "errors" "fmt" "os" "time" @@ -31,7 +30,11 @@ func newCmdUpgrade() *cobra.Command { cmd := &cobra.Command{ Use: "upgrade [flags]", Short: "Output Kubernetes configs to upgrade an existing Linkerd control plane", - Long: "Output Kubernetes configs to upgrade an existing Linkerd control plane.", + Long: `Output Kubernetes configs to upgrade an existing Linkerd control plane. + +Note that the default flag values for this command come from the Linkerd control +plane. The default values displayed in the Flags section below only apply to the +install command.`, RunE: func(cmd *cobra.Command, args []string) error { // We need a Kubernetes client to fetch configs and issuer secrets. k, err := options.newK8s() @@ -50,6 +53,9 @@ func newCmdUpgrade() *cobra.Command { // We recorded flags during a prior install. If we haven't overridden the // flag on this upgrade, reset that prior value as if it were specified now. + // + // This implies that the default flag values for the upgrade command come + // from the control-plane, and not from the defaults specified in the FlagSet. setOptionsFromInstall(flags, configs.GetInstall()) if err = options.validate(); err != nil { @@ -100,7 +106,7 @@ func setOptionsFromInstall(flags *pflag.FlagSet, install *pb.Install) { func (options *upgradeOptions) newK8s() (*kubernetes.Clientset, error) { if options.ignoreCluster { - return nil, errors.New("--ignore-cluster cannot be used with upgrade") + panic("ignore cluster must be unset") // Programmer error. } c, err := k8s.GetConfig(kubeconfigPath, kubeContext) diff --git a/controller/cmd/identity/main.go b/controller/cmd/identity/main.go index 198f57fc45616..6738ae0411587 100644 --- a/controller/cmd/identity/main.go +++ b/controller/cmd/identity/main.go @@ -60,7 +60,10 @@ func main() { log.Fatalf("Failed to read trust anchors: %s", err) } - creds, err := tls.ReadPEMCreds(filepath.Join(*issuerPath, "key.pem"), filepath.Join(*issuerPath, "crt.pem")) + creds, err := tls.ReadPEMCreds( + filepath.Join(*issuerPath, consts.IdentityIssuerKeyName), + filepath.Join(*issuerPath, consts.IdentityIssuerCrtName), + ) if err != nil { log.Fatalf("Failed to read CA from %s: %s", *issuerPath, err) } From 70681015a261f234e067bef603209e30b2602dbc Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Mon, 1 Apr 2019 19:51:07 +0000 Subject: [PATCH 10/12] flagSet reorg --- cli/cmd/install.go | 8 ++++---- cli/cmd/upgrade.go | 4 ++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/cli/cmd/install.go b/cli/cmd/install.go index 67a46ea7e9996..7bd9d47bfd476 100644 --- a/cli/cmd/install.go +++ b/cli/cmd/install.go @@ -224,7 +224,7 @@ func newCmdInstall() *cobra.Command { cmd.PersistentFlags().AddFlagSet(flags) // Some flags are not available during upgrade, etc. - cmd.PersistentFlags().AddFlagSet(options.installOnlyFlagSet(pflag.ExitOnError)) + cmd.PersistentFlags().AddFlagSet(options.flagSet(pflag.ExitOnError)) return cmd } @@ -252,7 +252,7 @@ func (options *installOptions) validateAndBuild(flags *pflag.FlagSet) (*installV } // flagSet returns flags usable during install or upgrade. -func (options *installOptions) flagSet(e pflag.ErrorHandling) *pflag.FlagSet { +func (options *installOptions) baseFlagSet(e pflag.ErrorHandling) *pflag.FlagSet { flags := pflag.NewFlagSet("install", e) flags.AddFlagSet(options.proxyConfigOptions.flagSet(e)) @@ -294,8 +294,8 @@ func (options *installOptions) flagSet(e pflag.ErrorHandling) *pflag.FlagSet { } // installOnlyFlagSet includes flags that are only accessible at install-time and not at upgrade-time. -func (options *installOptions) installOnlyFlagSet(e pflag.ErrorHandling) *pflag.FlagSet { - flags := pflag.NewFlagSet("install", e) +func (options *installOptions) flagSet(e pflag.ErrorHandling) *pflag.FlagSet { + flags := options.baseFlagSet(e) flags.StringVar( &options.identityOptions.trustDomain, "identity-trust-domain", options.identityOptions.trustDomain, diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go index 2d38b083fcc6e..13d2f159376cd 100644 --- a/cli/cmd/upgrade.go +++ b/cli/cmd/upgrade.go @@ -104,6 +104,10 @@ func setOptionsFromInstall(flags *pflag.FlagSet, install *pb.Install) { } } +func (options *upgradeOptions) flagSet(e pflag.ErrorHandling) *pflag.FlagSet { + return options.baseFlagSet(e) +} + func (options *upgradeOptions) newK8s() (*kubernetes.Clientset, error) { if options.ignoreCluster { panic("ignore cluster must be unset") // Programmer error. From eead6d9f8b094378955057ddfe10c70a26fca743 Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Mon, 1 Apr 2019 20:08:39 +0000 Subject: [PATCH 11/12] clarify recordableFlagSet --- cli/cmd/install.go | 18 +++++++++++------- cli/cmd/upgrade.go | 6 +----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cli/cmd/install.go b/cli/cmd/install.go index 7bd9d47bfd476..b7635cc02b256 100644 --- a/cli/cmd/install.go +++ b/cli/cmd/install.go @@ -201,7 +201,10 @@ func newInstallIdentityOptionsWithDefaults() *installIdentityOptions { func newCmdInstall() *cobra.Command { options := newInstallOptionsWithDefaults() - flags := options.flagSet(pflag.ExitOnError) + + // The base flags are recorded separately s that they can be serialized into + // the configuration in validateAndBuild. + flags := options.recordableFlagSet(pflag.ExitOnError) cmd := &cobra.Command{ Use: "install [flags]", @@ -224,7 +227,7 @@ func newCmdInstall() *cobra.Command { cmd.PersistentFlags().AddFlagSet(flags) // Some flags are not available during upgrade, etc. - cmd.PersistentFlags().AddFlagSet(options.flagSet(pflag.ExitOnError)) + cmd.PersistentFlags().AddFlagSet(options.installOnlyFlagSet(pflag.ExitOnError)) return cmd } @@ -251,8 +254,8 @@ func (options *installOptions) validateAndBuild(flags *pflag.FlagSet) (*installV return values, configs, nil } -// flagSet returns flags usable during install or upgrade. -func (options *installOptions) baseFlagSet(e pflag.ErrorHandling) *pflag.FlagSet { +// recordableFlagSet returns flags usable during install or upgrade. +func (options *installOptions) recordableFlagSet(e pflag.ErrorHandling) *pflag.FlagSet { flags := pflag.NewFlagSet("install", e) flags.AddFlagSet(options.proxyConfigOptions.flagSet(e)) @@ -293,9 +296,10 @@ func (options *installOptions) baseFlagSet(e pflag.ErrorHandling) *pflag.FlagSet return flags } -// installOnlyFlagSet includes flags that are only accessible at install-time and not at upgrade-time. -func (options *installOptions) flagSet(e pflag.ErrorHandling) *pflag.FlagSet { - flags := options.baseFlagSet(e) +// installOnlyFlagSet includes flags that are only accessible at install-time +// and not at upgrade-time. +func (options *installOptions) installOnlyFlagSet(e pflag.ErrorHandling) *pflag.FlagSet { + flags := options.recordableFlagSet(e) flags.StringVar( &options.identityOptions.trustDomain, "identity-trust-domain", options.identityOptions.trustDomain, diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go index 13d2f159376cd..7b90c01305b4b 100644 --- a/cli/cmd/upgrade.go +++ b/cli/cmd/upgrade.go @@ -25,7 +25,7 @@ func newUpgradeOptionsWithDefaults() *upgradeOptions { func newCmdUpgrade() *cobra.Command { options := newUpgradeOptionsWithDefaults() - flags := options.flagSet(pflag.ExitOnError) + flags := options.recordableFlagSet(pflag.ExitOnError) cmd := &cobra.Command{ Use: "upgrade [flags]", @@ -104,10 +104,6 @@ func setOptionsFromInstall(flags *pflag.FlagSet, install *pb.Install) { } } -func (options *upgradeOptions) flagSet(e pflag.ErrorHandling) *pflag.FlagSet { - return options.baseFlagSet(e) -} - func (options *upgradeOptions) newK8s() (*kubernetes.Clientset, error) { if options.ignoreCluster { panic("ignore cluster must be unset") // Programmer error. From 374562edc0e24e74f5a0f4a3933e54e351db8a62 Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Mon, 1 Apr 2019 20:21:04 +0000 Subject: [PATCH 12/12] bump ci