From 6671d7ebd7a08639b891c62b18e3411d8d28b3ee Mon Sep 17 00:00:00 2001 From: Jeremy Jacobson Date: Fri, 21 Jul 2023 10:33:22 -0700 Subject: [PATCH] [CC-5718] Remove HCP token requirement during bootstrap (#18140) * [CC-5718] Remove HCP token requirement during bootstrap * Re-add error for loading HCP management token * Remove old comment * Add changelog entry * Remove extra validation line * Apply suggestions from code review Co-authored-by: lornasong --------- Co-authored-by: lornasong --- .changelog/18140.txt | 3 +++ agent/hcp/bootstrap/bootstrap.go | 25 +++++++++--------- agent/hcp/bootstrap/bootstrap_test.go | 37 +++++++++++++++------------ 3 files changed, 37 insertions(+), 28 deletions(-) create mode 100644 .changelog/18140.txt diff --git a/.changelog/18140.txt b/.changelog/18140.txt new file mode 100644 index 0000000000..fabd9fc291 --- /dev/null +++ b/.changelog/18140.txt @@ -0,0 +1,3 @@ +```release-note:improvement +hcp: Removes requirement for HCP to provide a management token +``` diff --git a/agent/hcp/bootstrap/bootstrap.go b/agent/hcp/bootstrap/bootstrap.go index 38fa7d2274..191859ea00 100644 --- a/agent/hcp/bootstrap/bootstrap.go +++ b/agent/hcp/bootstrap/bootstrap.go @@ -298,21 +298,25 @@ func persistAndProcessConfig(dataDir string, devMode bool, bsCfg *hcpclient.Boot return "", fmt.Errorf("failed to persist bootstrap config: %w", err) } - if err := validateManagementToken(bsCfg.ManagementToken); err != nil { - return "", fmt.Errorf("invalid management token: %w", err) - } - if err := persistManagementToken(dir, bsCfg.ManagementToken); err != nil { - return "", fmt.Errorf("failed to persist HCP management token: %w", err) + // HCP only returns the management token if it requires Consul to + // initialize it + if bsCfg.ManagementToken != "" { + if err := validateManagementToken(bsCfg.ManagementToken); err != nil { + return "", fmt.Errorf("invalid management token: %w", err) + } + if err := persistManagementToken(dir, bsCfg.ManagementToken); err != nil { + return "", fmt.Errorf("failed to persist HCP management token: %w", err) + } } - if err := persistSucessMarker(dir); err != nil { + if err := persistSuccessMarker(dir); err != nil { return "", fmt.Errorf("failed to persist success marker: %w", err) } } return cfgJSON, nil } -func persistSucessMarker(dir string) error { +func persistSuccessMarker(dir string) error { name := filepath.Join(dir, successFileName) return os.WriteFile(name, []byte(""), 0600) @@ -352,12 +356,9 @@ func persistTLSCerts(dir string, serverCert, serverKey string, caCerts []string) return nil } -// Basic validation to ensure a UUID was loaded. +// Basic validation to ensure a UUID was loaded and assumes the token is non-empty func validateManagementToken(token string) error { - if token == "" { - return errors.New("missing HCP management token") - } - + // note: we assume that the token is not an empty string if _, err := uuid.ParseUUID(token); err != nil { return errors.New("management token is not a valid UUID") } diff --git a/agent/hcp/bootstrap/bootstrap_test.go b/agent/hcp/bootstrap/bootstrap_test.go index 7cb46a32bf..74b57e5f50 100644 --- a/agent/hcp/bootstrap/bootstrap_test.go +++ b/agent/hcp/bootstrap/bootstrap_test.go @@ -305,9 +305,10 @@ func Test_loadPersistedBootstrapConfig(t *testing.T) { warning string } type testCase struct { - existingCluster bool - mutateFn func(t *testing.T, dir string) - expect expect + existingCluster bool + disableManagementToken bool + mutateFn func(t *testing.T, dir string) + expect expect } run := func(t *testing.T, tc testCase) { @@ -319,7 +320,7 @@ func Test_loadPersistedBootstrapConfig(t *testing.T) { // Do some common setup as if we received config from HCP and persisted it to disk. require.NoError(t, lib.EnsurePath(dir, true)) - require.NoError(t, persistSucessMarker(dir)) + require.NoError(t, persistSuccessMarker(dir)) if !tc.existingCluster { caCert, caKey, err := tlsutil.GenerateCA(tlsutil.CAOpts{}) @@ -333,9 +334,12 @@ func Test_loadPersistedBootstrapConfig(t *testing.T) { require.NoError(t, persistBootstrapConfig(dir, cfgJSON)) } - token, err := uuid.GenerateUUID() - require.NoError(t, err) - require.NoError(t, persistManagementToken(dir, token)) + var token string + if !tc.disableManagementToken { + token, err = uuid.GenerateUUID() + require.NoError(t, err) + require.NoError(t, persistManagementToken(dir, token)) + } // Optionally mutate the persisted data to trigger errors while loading. if tc.mutateFn != nil { @@ -348,7 +352,6 @@ func Test_loadPersistedBootstrapConfig(t *testing.T) { if loaded { require.Equal(t, token, cfg.ManagementToken) require.Empty(t, ui.ErrorWriter.String()) - } else { require.Nil(t, cfg) require.Contains(t, ui.ErrorWriter.String(), tc.expect.warning) @@ -365,15 +368,11 @@ func Test_loadPersistedBootstrapConfig(t *testing.T) { warning: "", }, }, - "existing cluster missing token": { - existingCluster: true, - mutateFn: func(t *testing.T, dir string) { - // Remove the token file while leaving the existing cluster marker. - require.NoError(t, os.Remove(filepath.Join(dir, tokenFileName))) - }, + "existing cluster no token": { + existingCluster: true, + disableManagementToken: true, expect: expect{ - loaded: false, - warning: "configuration files on disk are incomplete", + loaded: false, }, }, "existing cluster no files": { @@ -396,6 +395,12 @@ func Test_loadPersistedBootstrapConfig(t *testing.T) { warning: "", }, }, + "new cluster with no token": { + disableManagementToken: true, + expect: expect{ + loaded: false, + }, + }, "new cluster some files": { mutateFn: func(t *testing.T, dir string) { // Remove one of the required files