Skip to content

Commit

Permalink
Add safe accessor for Global in linkerd-config (linkerd#5269)
Browse files Browse the repository at this point in the history 
CLI crashes if linkerd-config contains unexpected values.

Add a safe accessor that initializes an empty Global on the first
access. Refactor all accesses to use the newly introduced accessor using
gopls.

Add test for linkerd-config data without Global.

Fixes linkerd#5215

Co-authored-by: Itai Schwartz <yitai27@gmail.com>
Signed-off-by: Hod Bin Noon <bin.noon.hod@gmail.com>
  • Loading branch information
@hodbn @itaischwartz
hodbn and itaischwartz authored Nov 23, 2020
1 parent ca86a31 commit 92eb174
Show file tree
Hide file tree
Showing 19 changed files with 345 additions and 313 deletions.
22 changes: 11 additions & 11 deletions cli/cmd/inject.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,17 +350,17 @@ func fetchConfigs(ctx context.Context) (*linkerd2.Values, error) {
func getOverrideAnnotations(values *charts.Values, base *charts.Values) map[string]string {
overrideAnnotations := make(map[string]string)

proxy := values.Global.Proxy
baseProxy := base.Global.Proxy
proxy := values.GetGlobal().Proxy
baseProxy := base.GetGlobal().Proxy
if proxy.Image.Version != baseProxy.Image.Version {
overrideAnnotations[k8s.ProxyVersionOverrideAnnotation] = proxy.Image.Version
}

if values.Global.ProxyInit.IgnoreInboundPorts != base.Global.ProxyInit.IgnoreInboundPorts {
overrideAnnotations[k8s.ProxyIgnoreInboundPortsAnnotation] = values.Global.ProxyInit.IgnoreInboundPorts
if values.GetGlobal().ProxyInit.IgnoreInboundPorts != base.GetGlobal().ProxyInit.IgnoreInboundPorts {
overrideAnnotations[k8s.ProxyIgnoreInboundPortsAnnotation] = values.GetGlobal().ProxyInit.IgnoreInboundPorts
}
if values.Global.ProxyInit.IgnoreOutboundPorts != base.Global.ProxyInit.IgnoreOutboundPorts {
overrideAnnotations[k8s.ProxyIgnoreOutboundPortsAnnotation] = values.Global.ProxyInit.IgnoreOutboundPorts
if values.GetGlobal().ProxyInit.IgnoreOutboundPorts != base.GetGlobal().ProxyInit.IgnoreOutboundPorts {
overrideAnnotations[k8s.ProxyIgnoreOutboundPortsAnnotation] = values.GetGlobal().ProxyInit.IgnoreOutboundPorts
}

if proxy.Ports.Admin != baseProxy.Ports.Admin {
Expand All @@ -379,15 +379,15 @@ func getOverrideAnnotations(values *charts.Values, base *charts.Values) map[stri
if proxy.Image.Name != baseProxy.Image.Name {
overrideAnnotations[k8s.ProxyImageAnnotation] = proxy.Image.Name
}
if values.Global.ProxyInit.Image.Name != base.Global.ProxyInit.Image.Name {
overrideAnnotations[k8s.ProxyInitImageAnnotation] = values.Global.ProxyInit.Image.Name
if values.GetGlobal().ProxyInit.Image.Name != base.GetGlobal().ProxyInit.Image.Name {
overrideAnnotations[k8s.ProxyInitImageAnnotation] = values.GetGlobal().ProxyInit.Image.Name
}
if values.DebugContainer.Image.Name != base.DebugContainer.Image.Name {
overrideAnnotations[k8s.DebugImageAnnotation] = values.DebugContainer.Image.Name
}

if values.Global.ProxyInit.Image.Version != base.Global.ProxyInit.Image.Version {
overrideAnnotations[k8s.ProxyInitImageVersionAnnotation] = values.Global.ProxyInit.Image.Version
if values.GetGlobal().ProxyInit.Image.Version != base.GetGlobal().ProxyInit.Image.Version {
overrideAnnotations[k8s.ProxyInitImageVersionAnnotation] = values.GetGlobal().ProxyInit.Image.Version
}

if values.DebugContainer.Image.Version != base.DebugContainer.Image.Version {
Expand Down Expand Up @@ -455,7 +455,7 @@ func getOverrideAnnotations(values *charts.Values, base *charts.Values) map[stri
}

// Set fields that can't be converted into annotations
values.Global.Namespace = controlPlaneNamespace
values.GetGlobal().Namespace = controlPlaneNamespace

return overrideAnnotations
}
Expand Down
66 changes: 33 additions & 33 deletions cli/cmd/inject_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ func defaultConfig() *linkerd2.Values {
if err != nil {
log.Fatalf("Unexpected error: %v", err)
}
defaultConfig.Global.LinkerdVersion = "test-inject-control-plane-version"
defaultConfig.Global.Proxy.Image.Version = "test-inject-proxy-version"
defaultConfig.GetGlobal().LinkerdVersion = "test-inject-control-plane-version"
defaultConfig.GetGlobal().Proxy.Image.Version = "test-inject-proxy-version"
defaultConfig.DebugContainer.Image.Version = "test-inject-debug-version"

return defaultConfig
Expand All @@ -74,10 +74,10 @@ func TestUninjectAndInject(t *testing.T) {
defaultValues := defaultConfig()

overrideConfig := defaultConfig()
overrideConfig.Global.Proxy.Image.Version = "override"
overrideConfig.GetGlobal().Proxy.Image.Version = "override"

proxyResourceConfig := defaultConfig()
proxyResourceConfig.Global.Proxy.Resources = &linkerd2.Resources{
proxyResourceConfig.GetGlobal().Proxy.Resources = &linkerd2.Resources{
CPU: linkerd2.Constraints{
Request: "110m",
Limit: "160m",
Expand All @@ -89,11 +89,11 @@ func TestUninjectAndInject(t *testing.T) {
}

cniEnabledConfig := defaultConfig()
cniEnabledConfig.Global.CNIEnabled = true
cniEnabledConfig.GetGlobal().CNIEnabled = true

proxyIgnorePortsConfig := defaultConfig()
proxyIgnorePortsConfig.Global.ProxyInit.IgnoreInboundPorts = "22,8100-8102"
proxyIgnorePortsConfig.Global.ProxyInit.IgnoreOutboundPorts = "5432"
proxyIgnorePortsConfig.GetGlobal().ProxyInit.IgnoreInboundPorts = "22,8100-8102"
proxyIgnorePortsConfig.GetGlobal().ProxyInit.IgnoreOutboundPorts = "5432"

testCases := []testCase{
{
Expand All @@ -110,7 +110,7 @@ func TestUninjectAndInject(t *testing.T) {
injectProxy: false,
testInjectConfig: func() *linkerd2.Values {
values := defaultConfig()
values.Global.Proxy.Ports.Admin = 1234
values.GetGlobal().Proxy.Ports.Admin = 1234
return values
}(),
},
Expand All @@ -121,7 +121,7 @@ func TestUninjectAndInject(t *testing.T) {
injectProxy: true,
testInjectConfig: func() *linkerd2.Values {
values := defaultConfig()
values.Global.Proxy.Ports.Admin = 1234
values.GetGlobal().Proxy.Ports.Admin = 1234
return values
}(),
},
Expand Down Expand Up @@ -295,8 +295,8 @@ func TestUninjectAndInject(t *testing.T) {
injectProxy: true,
testInjectConfig: func() *linkerd2.Values {
values := defaultConfig()
values.Global.Proxy.Trace.CollectorSvcAddr = "linkerd-collector"
values.Global.Proxy.Trace.CollectorSvcAccount = "linkerd-collector.linkerd"
values.GetGlobal().Proxy.Trace.CollectorSvcAddr = "linkerd-collector"
values.GetGlobal().Proxy.Trace.CollectorSvcAccount = "linkerd-collector.linkerd"
return values
}(),
},
Expand Down Expand Up @@ -328,7 +328,7 @@ func testInjectCmd(t *testing.T, tc injectCmd) {
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
testConfig.Global.Proxy.Image.Version = "testinjectversion"
testConfig.GetGlobal().Proxy.Image.Version = "testinjectversion"

errBuffer := &bytes.Buffer{}
outBuffer := &bytes.Buffer{}
Expand Down Expand Up @@ -595,25 +595,25 @@ func TestProxyConfigurationAnnotations(t *testing.T) {
if err != nil {
t.Fatal(err)
}
values.Global.ProxyInit.IgnoreInboundPorts = "8500-8505"
values.Global.ProxyInit.IgnoreOutboundPorts = "3306"
values.Global.Proxy.Ports.Admin = 1234
values.Global.Proxy.Ports.Control = 4191
values.Global.Proxy.Ports.Inbound = 4144
values.Global.Proxy.Ports.Outbound = 4141
values.Global.Proxy.UID = 999
values.Global.Proxy.LogLevel = "debug"
values.Global.Proxy.LogFormat = "cool"
values.Global.Proxy.DisableIdentity = true
values.Global.Proxy.DisableTap = true
values.Global.Proxy.EnableExternalProfiles = true
values.Global.Proxy.Resources.CPU.Request = "10m"
values.Global.Proxy.Resources.CPU.Limit = "100m"
values.Global.Proxy.Resources.Memory.Request = "10Mi"
values.Global.Proxy.Resources.Memory.Limit = "50Mi"
values.Global.Proxy.Trace.CollectorSvcAddr = "oc-collector.tracing:55678"
values.Global.Proxy.Trace.CollectorSvcAccount = "svcAccount"
values.Global.Proxy.WaitBeforeExitSeconds = 10
values.GetGlobal().ProxyInit.IgnoreInboundPorts = "8500-8505"
values.GetGlobal().ProxyInit.IgnoreOutboundPorts = "3306"
values.GetGlobal().Proxy.Ports.Admin = 1234
values.GetGlobal().Proxy.Ports.Control = 4191
values.GetGlobal().Proxy.Ports.Inbound = 4144
values.GetGlobal().Proxy.Ports.Outbound = 4141
values.GetGlobal().Proxy.UID = 999
values.GetGlobal().Proxy.LogLevel = "debug"
values.GetGlobal().Proxy.LogFormat = "cool"
values.GetGlobal().Proxy.DisableIdentity = true
values.GetGlobal().Proxy.DisableTap = true
values.GetGlobal().Proxy.EnableExternalProfiles = true
values.GetGlobal().Proxy.Resources.CPU.Request = "10m"
values.GetGlobal().Proxy.Resources.CPU.Limit = "100m"
values.GetGlobal().Proxy.Resources.Memory.Request = "10Mi"
values.GetGlobal().Proxy.Resources.Memory.Limit = "50Mi"
values.GetGlobal().Proxy.Trace.CollectorSvcAddr = "oc-collector.tracing:55678"
values.GetGlobal().Proxy.Trace.CollectorSvcAccount = "svcAccount"
values.GetGlobal().Proxy.WaitBeforeExitSeconds = 10

expectedOverrides := map[string]string{
k8s.ProxyIgnoreInboundPortsAnnotation: "8500-8505",
Expand Down Expand Up @@ -651,7 +651,7 @@ func TestProxyImageAnnotations(t *testing.T) {
if err != nil {
t.Fatal(err)
}
values.Global.Proxy.Image = &linkerd2.Image{
values.GetGlobal().Proxy.Image = &linkerd2.Image{
Name: "my.registry/linkerd/proxy",
Version: "test-proxy-version",
PullPolicy: "Always",
Expand All @@ -677,7 +677,7 @@ func TestProxyInitImageAnnotations(t *testing.T) {
if err != nil {
t.Fatal(err)
}
values.Global.ProxyInit.Image = &linkerd2.Image{
values.GetGlobal().ProxyInit.Image = &linkerd2.Image{
Name: "my.registry/linkerd/proxy-init",
Version: "test-proxy-init-version",
}
Expand Down
2 changes: 1 addition & 1 deletion cli/cmd/install-sp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ support, verify "kubectl api-versions" outputs "linkerd.io/v1alpha2".`,
return err
}

clusterDomain := values.Global.ClusterDomain
clusterDomain := values.GetGlobal().ClusterDomain
if clusterDomain == "" {
clusterDomain = defaultClusterDomain
}
Expand Down
8 changes: 4 additions & 4 deletions cli/cmd/install.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ resources for the Linkerd control plane. This command should be followed by
}
if !ignoreCluster {
// Ensure k8s is reachable and that Linkerd is not already installed.
if err := errAfterRunningChecks(values.Global.CNIEnabled); err != nil {
if err := errAfterRunningChecks(values.GetGlobal().CNIEnabled); err != nil {
if healthcheck.IsCategoryError(err, healthcheck.KubernetesAPIChecks) {
fmt.Fprintf(os.Stderr, errMsgCannotInitializeClient, err)
} else {
Expand Down Expand Up @@ -173,7 +173,7 @@ control plane. It should be run after "linkerd install config".`,
if !skipChecks {
// check if global resources exist to determine if the `install config`
// stage succeeded
if err := errAfterRunningChecks(values.Global.CNIEnabled); err == nil {
if err := errAfterRunningChecks(values.GetGlobal().CNIEnabled); err == nil {
if healthcheck.IsCategoryError(err, healthcheck.KubernetesAPIChecks) {
fmt.Fprintf(os.Stderr, errMsgCannotInitializeClient, err)
} else {
Expand Down Expand Up @@ -296,7 +296,7 @@ func install(ctx context.Context, w io.Writer, values *l5dcharts.Values, flags [
func render(w io.Writer, values *l5dcharts.Values, stage string) error {

// Set any global flags if present, common with install and upgrade
values.Global.Namespace = controlPlaneNamespace
values.GetGlobal().Namespace = controlPlaneNamespace

// Render raw values and create chart config
rawValues, err := yaml.Marshal(values)
Expand Down Expand Up @@ -384,7 +384,7 @@ func render(w io.Writer, values *l5dcharts.Values, stage string) error {
}

if stage == "" || stage == controlPlaneStage {
overrides, err := renderOverrides(values, values.Global.Namespace)
overrides, err := renderOverrides(values, values.GetGlobal().Namespace)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions cli/cmd/install_helm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,8 @@ func readTestValues(ha bool, ignoreOutboundPorts string, ignoreInboundPorts stri
if err != nil {
return nil, err
}
values.Global.ProxyInit.IgnoreOutboundPorts = ignoreOutboundPorts
values.Global.ProxyInit.IgnoreInboundPorts = ignoreInboundPorts
values.GetGlobal().ProxyInit.IgnoreOutboundPorts = ignoreOutboundPorts
values.GetGlobal().ProxyInit.IgnoreInboundPorts = ignoreInboundPorts

return values, nil
}
Expand Down
Loading

0 comments on commit 92eb174

Please sign in to comment.