From 81820d3fc43152606e8b48f07a41d3ccbb7e64b7 Mon Sep 17 00:00:00 2001 From: "Tengjiao (TJ) Chen" Date: Tue, 4 Apr 2023 20:14:23 -0700 Subject: [PATCH 1/4] * Support CA bundle and identity cert/key authentication * Change the helm adjust the new arguments and environment variable inputs --- charts/member-agent/templates/deployment.yaml | 22 +++++-- charts/member-agent/values.yaml | 4 ++ cmd/memberagent/main.go | 57 ++++++++++++++----- 3 files changed, 66 insertions(+), 17 deletions(-) diff --git a/charts/member-agent/templates/deployment.yaml b/charts/member-agent/templates/deployment.yaml index cfa1a1468..a08cad507 100644 --- a/charts/member-agent/templates/deployment.yaml +++ b/charts/member-agent/templates/deployment.yaml @@ -25,7 +25,11 @@ spec: containerPort: 80 args: - --leader-elect=true + {{- if .Values.useCaAuth }} + - --use-ca-auth={{ .Values.useCaAuth }} + {{- else }} - --tls-insecure={{ .Values.tlsClientInsecure }} + {{- end }} - --v={{ .Values.logVerbosity }} - -add_dir_header env: @@ -37,6 +41,14 @@ spec: value: "{{ .Values.config.memberClusterName }}" - name: HUB_CERTIFICATE_AUTHORITY value: "{{ .Values.config.hubCA }}" + {{- if .Values.useCaAuth }} + - name: IDENTITY_KEY + value: "{{ .Values.config.identityKey }}" + - name: IDENTITY_CERT + value: "{{ .Values.config.identityCert }}" + - name: CA_BUNDLE + value: "{{ .Values.config.CABundle }}" + {{- end }} resources: {{- toYaml .Values.resources | nindent 12 }} ports: @@ -63,15 +75,16 @@ spec: volumeMounts: - name: provider-token mountPath: /config + {{- if not .Values.useCaAuth }} - name: refresh-token image: "{{ .Values.refreshtoken.repository }}:{{ .Values.refreshtoken.tag }}" imagePullPolicy: {{ .Values.refreshtoken.pullPolicy }} args: - {{ $provider := .Values.config.provider }} + {{- $provider := .Values.config.provider }} - {{ $provider }} - {{ range $key, $value := (index .Values $provider) }} + {{- range $key, $value := (index .Values $provider) }} - --{{ $key }}={{ $value }} - {{ end }} + {{- end }} - --v={{ .Values.logVerbosity }} ports: - name: http @@ -80,7 +93,8 @@ spec: {{- toYaml .Values.resources | nindent 12 }} volumeMounts: - name: provider-token - mountPath: /config + mountPath: /config + {{- end }} volumes: - name: provider-token emptyDir: {} diff --git a/charts/member-agent/values.yaml b/charts/member-agent/values.yaml index 2071c9a86..f2247c236 100644 --- a/charts/member-agent/values.yaml +++ b/charts/member-agent/values.yaml @@ -32,6 +32,9 @@ config: hubURL : https://: memberClusterName: membercluster-sample hubCA: + identityKey: "identity-key-path" + identityCert: "identity-cert-path" + CABundle: "ca-bundle-path" secret: name: "hub-kubeconfig-secret" @@ -41,3 +44,4 @@ azure: clientid: tlsClientInsecure: true #TODO should be false in the production +useCaAuth: false diff --git a/cmd/memberagent/main.go b/cmd/memberagent/main.go index 24cf6edc2..286e9f4db 100644 --- a/cmd/memberagent/main.go +++ b/cmd/memberagent/main.go @@ -39,6 +39,7 @@ import ( var ( scheme = runtime.NewScheme() + useCaAuth = flag.Bool("use-ca-auth", false, "Use identity and CA bundle to authenticate the member agent.") tlsClientInsecure = flag.Bool("tls-insecure", false, "Enable TLSClientConfig.Insecure property. Enabling this will make the connection inSecure (should be 'true' for testing purpose only.)") hubProbeAddr = flag.String("hub-health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") hubMetricsAddr = flag.String("hub-metrics-bind-address", ":8080", "The address the metric endpoint binds to.") @@ -72,7 +73,7 @@ func main() { } tokenFilePath := os.Getenv("CONFIG_PATH") - if tokenFilePath == "" { + if !*useCaAuth && tokenFilePath == "" { klog.ErrorS(errors.New("hub token file path cannot be empty"), "error has occurred retrieving CONFIG_PATH") os.Exit(1) } @@ -85,21 +86,51 @@ func main() { mcNamespace := fmt.Sprintf(utils.NamespaceNameFormat, mcName) - err := retry.OnError(retry.DefaultRetry, func(e error) bool { - return true - }, func() error { - // Stat returns file info. It will return - // an error if there is no file. - _, err := os.Stat(tokenFilePath) - return err - }) - if err != nil { - klog.ErrorS(err, " cannot retrieve token file from the path %s", tokenFilePath) - os.Exit(1) + identityKeyFile := os.Getenv("IDENTITY_KEY") + identityCertFile := os.Getenv("IDENTITY_CERT") + cABundleFile := os.Getenv("CA_BUNDLE") + + if *useCaAuth { + if identityKeyFile == "" { + klog.ErrorS(errors.New("identity key file path cannot be empty"), "error has occurred retrieving IDENTITY_KEY") + os.Exit(1) + } + + if identityCertFile == "" { + klog.ErrorS(errors.New("identity cert file path cannot be empty"), "error has occurred retrieving IDENTITY_CERT") + os.Exit(1) + } + + if cABundleFile == "" { + klog.ErrorS(errors.New("CA bundle file path cannot be empty"), "error has occurred retrieving CA_BUNDLE") + os.Exit(1) + } + } else { + err := retry.OnError(retry.DefaultRetry, func(e error) bool { + return true + }, func() error { + // Stat returns file info. It will return + // an error if there is no file. + _, err := os.Stat(tokenFilePath) + return err + }) + if err != nil { + klog.ErrorS(err, " cannot retrieve token file from the path %s", tokenFilePath) + os.Exit(1) + } } var hubConfig rest.Config - if *tlsClientInsecure { + if *useCaAuth { + hubConfig = rest.Config{ + Host: hubURL, + TLSClientConfig: rest.TLSClientConfig{ + CertFile: identityCertFile, + KeyFile: identityKeyFile, + CAFile: cABundleFile, + }, + } + } else if *tlsClientInsecure { hubConfig = rest.Config{ BearerTokenFile: tokenFilePath, Host: hubURL, From d21ff52798199d8a09ec3e5870365e2ae359983e Mon Sep 17 00:00:00 2001 From: "Tengjiao (TJ) Chen" Date: Thu, 6 Apr 2023 13:28:14 -0700 Subject: [PATCH 2/4] * Address the comments from Sean * change useCaAuth to useCAAuth * Edit the error msg for the token missing to reflect the CA auth logic --- charts/member-agent/templates/deployment.yaml | 8 ++++---- charts/member-agent/values.yaml | 2 +- cmd/memberagent/main.go | 10 +++++----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/charts/member-agent/templates/deployment.yaml b/charts/member-agent/templates/deployment.yaml index a08cad507..69d5f25b9 100644 --- a/charts/member-agent/templates/deployment.yaml +++ b/charts/member-agent/templates/deployment.yaml @@ -25,8 +25,8 @@ spec: containerPort: 80 args: - --leader-elect=true - {{- if .Values.useCaAuth }} - - --use-ca-auth={{ .Values.useCaAuth }} + {{- if .Values.useCAAuth }} + - --use-ca-auth={{ .Values.useCAAuth }} {{- else }} - --tls-insecure={{ .Values.tlsClientInsecure }} {{- end }} @@ -41,7 +41,7 @@ spec: value: "{{ .Values.config.memberClusterName }}" - name: HUB_CERTIFICATE_AUTHORITY value: "{{ .Values.config.hubCA }}" - {{- if .Values.useCaAuth }} + {{- if .Values.useCAAuth }} - name: IDENTITY_KEY value: "{{ .Values.config.identityKey }}" - name: IDENTITY_CERT @@ -75,7 +75,7 @@ spec: volumeMounts: - name: provider-token mountPath: /config - {{- if not .Values.useCaAuth }} + {{- if not .Values.useCAAuth }} - name: refresh-token image: "{{ .Values.refreshtoken.repository }}:{{ .Values.refreshtoken.tag }}" imagePullPolicy: {{ .Values.refreshtoken.pullPolicy }} diff --git a/charts/member-agent/values.yaml b/charts/member-agent/values.yaml index f2247c236..9f84e43b2 100644 --- a/charts/member-agent/values.yaml +++ b/charts/member-agent/values.yaml @@ -44,4 +44,4 @@ azure: clientid: tlsClientInsecure: true #TODO should be false in the production -useCaAuth: false +useCAAuth: false diff --git a/cmd/memberagent/main.go b/cmd/memberagent/main.go index 286e9f4db..07bd4845e 100644 --- a/cmd/memberagent/main.go +++ b/cmd/memberagent/main.go @@ -39,7 +39,7 @@ import ( var ( scheme = runtime.NewScheme() - useCaAuth = flag.Bool("use-ca-auth", false, "Use identity and CA bundle to authenticate the member agent.") + useCAAuth = flag.Bool("use-ca-auth", false, "Use identity and CA bundle to authenticate the member agent.") tlsClientInsecure = flag.Bool("tls-insecure", false, "Enable TLSClientConfig.Insecure property. Enabling this will make the connection inSecure (should be 'true' for testing purpose only.)") hubProbeAddr = flag.String("hub-health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") hubMetricsAddr = flag.String("hub-metrics-bind-address", ":8080", "The address the metric endpoint binds to.") @@ -73,8 +73,8 @@ func main() { } tokenFilePath := os.Getenv("CONFIG_PATH") - if !*useCaAuth && tokenFilePath == "" { - klog.ErrorS(errors.New("hub token file path cannot be empty"), "error has occurred retrieving CONFIG_PATH") + if !*useCAAuth && tokenFilePath == "" { + klog.ErrorS(errors.New("hub token file path cannot be empty if CA auth not used"), "error has occurred retrieving CONFIG_PATH") os.Exit(1) } @@ -90,7 +90,7 @@ func main() { identityCertFile := os.Getenv("IDENTITY_CERT") cABundleFile := os.Getenv("CA_BUNDLE") - if *useCaAuth { + if *useCAAuth { if identityKeyFile == "" { klog.ErrorS(errors.New("identity key file path cannot be empty"), "error has occurred retrieving IDENTITY_KEY") os.Exit(1) @@ -121,7 +121,7 @@ func main() { } var hubConfig rest.Config - if *useCaAuth { + if *useCAAuth { hubConfig = rest.Config{ Host: hubURL, TLSClientConfig: rest.TLSClientConfig{ From eb05499ca749d470e5518863c6cd6f6bb360ff90 Mon Sep 17 00:00:00 2001 From: "Tengjiao (TJ) Chen" Date: Fri, 7 Apr 2023 09:48:57 -0700 Subject: [PATCH 3/4] * Change the `cABundleFile` to `caBundleFile` as Zhiyin suggested --- cmd/memberagent/main.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/memberagent/main.go b/cmd/memberagent/main.go index 07bd4845e..f6e2a06b4 100644 --- a/cmd/memberagent/main.go +++ b/cmd/memberagent/main.go @@ -88,7 +88,7 @@ func main() { identityKeyFile := os.Getenv("IDENTITY_KEY") identityCertFile := os.Getenv("IDENTITY_CERT") - cABundleFile := os.Getenv("CA_BUNDLE") + caBundleFile := os.Getenv("CA_BUNDLE") if *useCAAuth { if identityKeyFile == "" { @@ -101,7 +101,7 @@ func main() { os.Exit(1) } - if cABundleFile == "" { + if caBundleFile == "" { klog.ErrorS(errors.New("CA bundle file path cannot be empty"), "error has occurred retrieving CA_BUNDLE") os.Exit(1) } @@ -127,7 +127,7 @@ func main() { TLSClientConfig: rest.TLSClientConfig{ CertFile: identityCertFile, KeyFile: identityKeyFile, - CAFile: cABundleFile, + CAFile: caBundleFile, }, } } else if *tlsClientInsecure { From 403acc88c2c8eca86dfa8e3bb632515ce2da1e80 Mon Sep 17 00:00:00 2001 From: "Tengjiao (TJ) Chen" Date: Sun, 9 Apr 2023 20:34:00 -0700 Subject: [PATCH 4/4] * Address the comments from Zhiying --- charts/member-agent/templates/deployment.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/charts/member-agent/templates/deployment.yaml b/charts/member-agent/templates/deployment.yaml index 69d5f25b9..92a8e653c 100644 --- a/charts/member-agent/templates/deployment.yaml +++ b/charts/member-agent/templates/deployment.yaml @@ -72,10 +72,10 @@ spec: httpGet: path: /readyz port: hubhealthz + {{- if not .Values.useCAAuth }} volumeMounts: - name: provider-token mountPath: /config - {{- if not .Values.useCAAuth }} - name: refresh-token image: "{{ .Values.refreshtoken.repository }}:{{ .Values.refreshtoken.tag }}" imagePullPolicy: {{ .Values.refreshtoken.pullPolicy }} @@ -94,10 +94,10 @@ spec: volumeMounts: - name: provider-token mountPath: /config - {{- end }} volumes: - name: provider-token emptyDir: {} + {{- end }} {{- with .Values.nodeSelector }} nodeSelector: {{- toYaml . | nindent 8 }}