From 9c5bb4ec0c7773a3e2e221a78f7500c04c5f8f8d Mon Sep 17 00:00:00 2001 From: Ivan Sim <1330522+ihcsim@users.noreply.github.com> Date: Tue, 26 Mar 2019 14:21:22 -0700 Subject: [PATCH] Convert CLI inject proxy options to annotations (#2547) * Include the DisableExternalProfile option even if it's 'false'. The override logic depends on this option to assign different profile suffix. * Check for proxy and init image overrides even when registry option is empty * Append the config annotations to the pod's meta before creating the patch. This ensures that any configs provided via the CLI options are persisted as annotations before the configs override. * Persist linkerd version CLI option Signed-off-by: Ivan Sim --- cli/cmd/inject.go | 95 ++++++++++++++++++++++------ cli/cmd/inject_test.go | 17 +++-- controller/proxy-injector/webhook.go | 5 +- pkg/inject/inject.go | 42 +++++++++--- pkg/inject/patch.go | 6 -- 5 files changed, 125 insertions(+), 40 deletions(-) diff --git a/cli/cmd/inject.go b/cli/cmd/inject.go index bdb4b5519..30ae4db06 100644 --- a/cli/cmd/inject.go +++ b/cli/cmd/inject.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "os" + "strconv" "strings" jsonpatch "github.com/evanphx/json-patch" @@ -34,15 +35,13 @@ type injectOptions struct { } type resourceTransformerInject struct { - configs *config.All - + configs *config.All + overrideAnnotations map[string]string proxyOutboundCapacity map[string]uint } -func runInjectCmd(inputs []io.Reader, errWriter, outWriter io.Writer, conf *config.All) int { - return transformInput(inputs, errWriter, outWriter, resourceTransformerInject{ - configs: conf, - }) +func runInjectCmd(inputs []io.Reader, errWriter, outWriter io.Writer, transformer *resourceTransformerInject) int { + return transformInput(inputs, errWriter, outWriter, transformer) } func newInjectOptions() *injectOptions { @@ -89,9 +88,14 @@ sub-folders, or coming from stdin.`, if err != nil { return err } + overrideAnnotations := map[string]string{} + options.overrideConfigs(configs, overrideAnnotations) - options.overrideConfigs(configs) - exitCode := uninjectAndInject(in, stderr, stdout, configs) + transformer := &resourceTransformerInject{ + configs: configs, + overrideAnnotations: overrideAnnotations, + } + exitCode := uninjectAndInject(in, stderr, stdout, transformer) os.Exit(exitCode) return nil }, @@ -106,12 +110,12 @@ sub-folders, or coming from stdin.`, return cmd } -func uninjectAndInject(inputs []io.Reader, errWriter, outWriter io.Writer, conf *config.All) int { +func uninjectAndInject(inputs []io.Reader, errWriter, outWriter io.Writer, transformer *resourceTransformerInject) int { var out bytes.Buffer - if exitCode := runUninjectSilentCmd(inputs, errWriter, &out, conf); exitCode != 0 { + if exitCode := runUninjectSilentCmd(inputs, errWriter, &out, transformer.configs); exitCode != 0 { return exitCode } - return runInjectCmd([]io.Reader{&out}, errWriter, outWriter, conf) + return runInjectCmd([]io.Reader{&out}, errWriter, outWriter, transformer) } func (rt resourceTransformerInject) transform(bytes []byte) ([]byte, []inject.Report, error) { @@ -127,6 +131,14 @@ func (rt resourceTransformerInject) transform(bytes []byte) ([]byte, []inject.Re r := inject.Report{UnsupportedResource: true} return bytes, []inject.Report{r}, nil } + + conf.AppendPodAnnotations(map[string]string{ + k8s.CreatedByAnnotation: k8s.CreatedByAnnotationValue(), + }) + if len(rt.overrideAnnotations) > 0 { + conf.AppendPodAnnotations(rt.overrideAnnotations) + } + p, reports, err := conf.GetPatch(bytes, inject.ShouldInjectCLI) if err != nil { return nil, nil, err @@ -134,7 +146,7 @@ func (rt resourceTransformerInject) transform(bytes []byte) ([]byte, []inject.Re if p.IsEmpty() { return bytes, reports, nil } - p.AddCreatedByPodAnnotation(k8s.CreatedByAnnotationValue()) + patchJSON, err := p.Marshal() if err != nil { return nil, nil, err @@ -281,61 +293,95 @@ func (options *injectOptions) fetchConfigsOrDefault() (*config.All, error) { return api.Config(context.Background(), &public.Empty{}) } -// overrideConfigs uses command-line overrides to update the provided configs -func (options *injectOptions) overrideConfigs(configs *config.All) { +// 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) { + if options.linkerdVersion != "" { + configs.Global.Version = options.linkerdVersion + } + if len(options.ignoreInboundPorts) > 0 { configs.Proxy.IgnoreInboundPorts = toPorts(options.ignoreInboundPorts) + overrideAnnotations[k8s.ProxyIgnoreInboundPortsAnnotation] = parsePorts(configs.Proxy.IgnoreInboundPorts) } if len(options.ignoreOutboundPorts) > 0 { configs.Proxy.IgnoreOutboundPorts = toPorts(options.ignoreOutboundPorts) + overrideAnnotations[k8s.ProxyIgnoreOutboundPortsAnnotation] = parsePorts(configs.Proxy.IgnoreOutboundPorts) } if options.proxyAdminPort != 0 { configs.Proxy.AdminPort = toPort(options.proxyAdminPort) + overrideAnnotations[k8s.ProxyAdminPortAnnotation] = parsePort(configs.Proxy.AdminPort) } if options.proxyControlPort != 0 { configs.Proxy.ControlPort = toPort(options.proxyControlPort) + overrideAnnotations[k8s.ProxyControlPortAnnotation] = parsePort(configs.Proxy.ControlPort) } if options.proxyInboundPort != 0 { configs.Proxy.InboundPort = toPort(options.proxyInboundPort) + overrideAnnotations[k8s.ProxyInboundPortAnnotation] = parsePort(configs.Proxy.InboundPort) } if options.proxyOutboundPort != 0 { configs.Proxy.OutboundPort = toPort(options.proxyOutboundPort) + overrideAnnotations[k8s.ProxyOutboundPortAnnotation] = parsePort(configs.Proxy.OutboundPort) } - if options.dockerRegistry != "" { - configs.Proxy.ProxyImage.ImageName = registryOverride(configs.Proxy.ProxyImage.ImageName, options.dockerRegistry) - configs.Proxy.ProxyInitImage.ImageName = registryOverride(configs.Proxy.ProxyInitImage.ImageName, options.dockerRegistry) + if options.proxyImage != "" { + configs.Proxy.ProxyImage.ImageName = options.proxyImage + if options.dockerRegistry != "" { + configs.Proxy.ProxyImage.ImageName = registryOverride(configs.Proxy.ProxyImage.ImageName, options.dockerRegistry) + } + overrideAnnotations[k8s.ProxyImageAnnotation] = configs.Proxy.ProxyImage.ImageName + } + + if options.initImage != "" { + configs.Proxy.ProxyInitImage.ImageName = options.initImage + if options.dockerRegistry != "" { + configs.Proxy.ProxyInitImage.ImageName = registryOverride(configs.Proxy.ProxyInitImage.ImageName, options.dockerRegistry) + } + overrideAnnotations[k8s.ProxyInitImageAnnotation] = configs.Proxy.ProxyInitImage.ImageName } if options.imagePullPolicy != "" { configs.Proxy.ProxyImage.PullPolicy = options.imagePullPolicy configs.Proxy.ProxyInitImage.PullPolicy = options.imagePullPolicy + overrideAnnotations[k8s.ProxyImagePullPolicyAnnotation] = options.imagePullPolicy } if options.proxyUID != 0 { configs.Proxy.ProxyUid = options.proxyUID + overrideAnnotations[k8s.ProxyUIDAnnotation] = strconv.FormatInt(options.proxyUID, 10) } if options.proxyLogLevel != "" { configs.Proxy.LogLevel = &config.LogLevel{Level: options.proxyLogLevel} + overrideAnnotations[k8s.ProxyLogLevelAnnotation] = options.proxyLogLevel } + // keep track of this option because its true/false value results in different + // values being assigned to the LINKERD2_PROXY_DESTINATION_PROFILE_SUFFIXES + // env var. Its annotation is added only if its value is true. + configs.Proxy.DisableExternalProfiles = options.disableExternalProfiles if options.disableExternalProfiles { - configs.Proxy.DisableExternalProfiles = true + overrideAnnotations[k8s.ProxyDisableExternalProfilesAnnotation] = "true" } if options.proxyCPURequest != "" { configs.Proxy.Resource.RequestCpu = options.proxyCPURequest + overrideAnnotations[k8s.ProxyCPURequestAnnotation] = options.proxyCPURequest } if options.proxyCPULimit != "" { configs.Proxy.Resource.LimitCpu = options.proxyCPULimit + overrideAnnotations[k8s.ProxyCPULimitAnnotation] = options.proxyCPULimit } if options.proxyMemoryRequest != "" { configs.Proxy.Resource.RequestMemory = options.proxyMemoryRequest + overrideAnnotations[k8s.ProxyMemoryRequestAnnotation] = options.proxyMemoryRequest } if options.proxyMemoryLimit != "" { configs.Proxy.Resource.LimitMemory = options.proxyMemoryLimit + overrideAnnotations[k8s.ProxyMemoryLimitAnnotation] = options.proxyMemoryLimit } } @@ -343,6 +389,10 @@ func toPort(p uint) *config.Port { return &config.Port{Port: uint32(p)} } +func parsePort(port *config.Port) string { + return strconv.FormatUint(uint64(port.GetPort()), 10) +} + func toPorts(ints []uint) []*config.Port { ports := make([]*config.Port, len(ints)) for i, p := range ints { @@ -350,3 +400,12 @@ func toPorts(ints []uint) []*config.Port { } return ports } + +func parsePorts(ports []*config.Port) string { + var str string + for _, port := range ports { + str += parsePort(port) + "," + } + + return strings.TrimSuffix(str, ",") +} diff --git a/cli/cmd/inject_test.go b/cli/cmd/inject_test.go index 8a504811d..e721e6525 100644 --- a/cli/cmd/inject_test.go +++ b/cli/cmd/inject_test.go @@ -39,8 +39,12 @@ func testUninjectAndInject(t *testing.T, tc testCase) { output := new(bytes.Buffer) report := new(bytes.Buffer) + transformer := &resourceTransformerInject{ + configs: tc.testInjectConfig, + overrideAnnotations: map[string]string{}, + } - if exitCode := uninjectAndInject([]io.Reader{read}, report, output, tc.testInjectConfig); exitCode != 0 { + if exitCode := uninjectAndInject([]io.Reader{read}, report, output, transformer); exitCode != 0 { t.Errorf("Unexpected error injecting YAML: %v\n", report) } diffTestdata(t, tc.goldenFileName, output.String()) @@ -211,7 +215,8 @@ func testInjectCmd(t *testing.T, tc injectCmd) { t.Fatalf("Unexpected error: %v", err) } - exitCode := runInjectCmd([]io.Reader{in}, errBuffer, outBuffer, testConfig) + transformer := &resourceTransformerInject{configs: testConfig} + exitCode := runInjectCmd([]io.Reader{in}, errBuffer, outBuffer, transformer) if exitCode != tc.exitCode { t.Fatalf("Expected exit code to be %d but got: %d", tc.exitCode, exitCode) } @@ -268,8 +273,8 @@ func testInjectFilePath(t *testing.T, tc injectFilePath) { errBuf := &bytes.Buffer{} actual := &bytes.Buffer{} - configs := testInstallConfig() - if exitCode := runInjectCmd(in, errBuf, actual, configs); exitCode != 0 { + transformer := &resourceTransformerInject{configs: testInstallConfig()} + if exitCode := runInjectCmd(in, errBuf, actual, transformer); exitCode != 0 { t.Fatal("Unexpected error. Exit code from runInjectCmd: ", exitCode) } diffTestdata(t, tc.expectedFile, actual.String()) @@ -286,8 +291,8 @@ func testReadFromFolder(t *testing.T, resourceFolder string, expectedFolder stri errBuf := &bytes.Buffer{} actual := &bytes.Buffer{} - configs := testInstallConfig() - if exitCode := runInjectCmd(in, errBuf, actual, configs); exitCode != 0 { + transformer := &resourceTransformerInject{configs: testInstallConfig()} + if exitCode := runInjectCmd(in, errBuf, actual, transformer); exitCode != 0 { t.Fatal("Unexpected error. Exit code from runInjectCmd: ", exitCode) } diff --git a/controller/proxy-injector/webhook.go b/controller/proxy-injector/webhook.go index b493e4eb3..0413ce395 100644 --- a/controller/proxy-injector/webhook.go +++ b/controller/proxy-injector/webhook.go @@ -122,6 +122,9 @@ func (w *Webhook) inject(request *admissionv1beta1.AdmissionRequest) (*admission return admissionResponse, nil } + conf.AppendPodAnnotations(map[string]string{ + pkgK8s.CreatedByAnnotation: fmt.Sprintf("linkerd/proxy-injector %s", version.Version), + }) p, reports, err := conf.GetPatch(request.Object.Raw, inject.ShouldInjectWebhook) if err != nil { return nil, err @@ -131,8 +134,6 @@ func (w *Webhook) inject(request *admissionv1beta1.AdmissionRequest) (*admission return admissionResponse, nil } - p.AddCreatedByPodAnnotation(fmt.Sprintf("linkerd/proxy-injector %s", version.Version)) - patchJSON, err := p.Marshal() if err != nil { return nil, err diff --git a/pkg/inject/inject.go b/pkg/inject/inject.go index 5f1a6a9d1..807e44e50 100644 --- a/pkg/inject/inject.go +++ b/pkg/inject/inject.go @@ -95,9 +95,10 @@ type ResourceConfig struct { pod struct { // Meta is the pod's metadata. It's exported so that the YAML marshaling // will work in the ParseMeta() function. - Meta *metav1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` - labels map[string]string - spec *v1.PodSpec + Meta *metav1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` + labels map[string]string + annotations map[string]string + spec *v1.PodSpec } } @@ -110,6 +111,7 @@ func NewResourceConfig(configs *config.All) *ResourceConfig { config.pod.Meta = &metav1.ObjectMeta{} config.pod.labels = map[string]string{k8s.ControllerNSLabel: configs.GetGlobal().GetLinkerdNamespace()} + config.pod.annotations = map[string]string{} return config } @@ -141,6 +143,13 @@ func (conf *ResourceConfig) WithOwnerRetriever(f OwnerRetrieverFunc) *ResourceCo return conf } +// AppendPodAnnotations appends the given annotations to the pod spec in conf +func (conf *ResourceConfig) AppendPodAnnotations(annotations map[string]string) { + for annotation, value := range annotations { + conf.pod.annotations[annotation] = value + } +} + // YamlMarshalObj returns the yaml for the workload in conf func (conf *ResourceConfig) YamlMarshalObj() ([]byte, error) { return yaml.Marshal(conf.workload.obj) @@ -187,8 +196,8 @@ func (conf *ResourceConfig) GetPatch( if conf.pod.spec != nil { report.update(conf) if shouldInject(conf, report) { - conf.injectPodSpec(patch) conf.injectObjectMeta(patch) + conf.injectPodSpec(patch) } } else { report.UnsupportedResource = true @@ -367,6 +376,10 @@ func (conf *ResourceConfig) parse(bytes []byte) error { } } + if conf.pod.Meta.Annotations == nil { + conf.pod.Meta.Annotations = map[string]string{} + } + return nil } @@ -607,6 +620,14 @@ func (conf *ResourceConfig) injectObjectMeta(patch *Patch) { patch.addPodLabel(k, v) } } + + for k, v := range conf.pod.annotations { + patch.addPodAnnotation(k, v) + + // append any additional pod annotations to the pod's meta. + // for e.g., annotations that were converted from CLI inject options. + conf.pod.Meta.Annotations[k] = v + } } func (conf *ResourceConfig) getOverride(annotation string) string { @@ -820,13 +841,18 @@ func (conf *ResourceConfig) proxyLivenessProbe() *v1.Probe { } func (conf *ResourceConfig) proxyDestinationProfileSuffixes() string { - if overrides := conf.getOverride(k8s.ProxyDisableExternalProfilesAnnotation); overrides != "" { - disableExternalProfiles, err := strconv.ParseBool(overrides) - if err == nil && disableExternalProfiles { - return internalProfileSuffix + disableExternalProfiles := conf.configs.GetProxy().GetDisableExternalProfiles() + if override := conf.getOverride(k8s.ProxyDisableExternalProfilesAnnotation); override != "" { + value, err := strconv.ParseBool(override) + if err == nil { + disableExternalProfiles = value } } + if disableExternalProfiles { + return internalProfileSuffix + } + return defaultProfileSuffix } diff --git a/pkg/inject/patch.go b/pkg/inject/patch.go index 99f26512c..edba6e9c8 100644 --- a/pkg/inject/patch.go +++ b/pkg/inject/patch.go @@ -129,12 +129,6 @@ func (p *Patch) addPodAnnotation(key, value string) { }) } -// AddCreatedByPodAnnotation tags the pod so that we can tell apart injections -// from the CLI and the webhook -func (p *Patch) AddCreatedByPodAnnotation(s string) { - p.addPodAnnotation(k8s.CreatedByAnnotation, s) -} - // IsEmpty returns true if the patch doesn't contain any operations func (p *Patch) IsEmpty() bool { return len(p.patchOps) == 0