feat: remove org flag requirement for provisioners (#14722)

Author: f0ssel

enterprise/cli/provisionerdaemonstart.go

@@ -24,7 +24,6 @@ import (
 	"github.com/coder/coder/v2/cli/cliui"
 	"github.com/coder/coder/v2/cli/cliutil"
 	"github.com/coder/coder/v2/coderd/database"
-	"github.com/coder/coder/v2/coderd/provisionerkey"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/codersdk/drpc"
 	"github.com/coder/coder/v2/provisioner/terraform"
@@ -73,32 +72,30 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
 			interruptCtx, interruptCancel := inv.SignalNotifyContext(ctx, agpl.InterruptSignals...)
 			defer interruptCancel()
 
-			// This can fail to get the current organization
-			// if the client is not authenticated as a user,
-			// like when only PSK is provided.
-			// This will be cleaner once PSK is replaced
-			// with org scoped authentication tokens.
-			org, err := orgContext.Selected(inv, client)
-			if err != nil {
-				var cErr *codersdk.Error
-				if !errors.As(err, &cErr) || cErr.StatusCode() != http.StatusUnauthorized {
-					return xerrors.Errorf("current organization: %w", err)
-				}
+			orgID := uuid.Nil
+			if preSharedKey == "" && provisionerKey == "" {
+				// We can only select an organization if using user auth
+				org, err := orgContext.Selected(inv, client)
+				if err != nil {
+					var cErr *codersdk.Error
+					if !errors.As(err, &cErr) || cErr.StatusCode() != http.StatusUnauthorized {
+						return xerrors.Errorf("current organization: %w", err)
+					}
 
-				if preSharedKey == "" && provisionerKey == "" {
 					return xerrors.New("must provide a pre-shared key or provisioner key when not authenticated as a user")
 				}
 
-				org = codersdk.Organization{MinimalOrganization: codersdk.MinimalOrganization{ID: uuid.Nil}}
-				if orgContext.FlagSelect != "" {
-					// If we are using PSK, we can't fetch the organization
-					// to validate org name so we need the user to provide
-					// a valid organization ID.
-					orgID, err := uuid.Parse(orgContext.FlagSelect)
-					if err != nil {
-						return xerrors.New("must provide an org ID when not authenticated as a user and organization is specified")
-					}
-					org = codersdk.Organization{MinimalOrganization: codersdk.MinimalOrganization{ID: orgID}}
+				orgID = org.ID
+			} else if orgContext.FlagSelect != "" {
+				return xerrors.New("cannot provide --org value with --psk or --key flags")
+			}
+
+			if provisionerKey != "" {
+				if preSharedKey != "" {
+					return xerrors.New("cannot provide both provisioner key --key and pre-shared key --psk")
+				}
+				if len(rawTags) > 0 {
+					return xerrors.New("cannot provide tags when using provisioner key")
 				}
 			}
 
@@ -115,19 +112,6 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
 				return err
 			}
 
-			if provisionerKey != "" {
-				if preSharedKey != "" {
-					return xerrors.New("cannot provide both provisioner key --key and pre-shared key --psk")
-				}
-				if len(rawTags) > 0 {
-					return xerrors.New("cannot provide tags when using provisioner key")
-				}
-				err = provisionerkey.Validate(provisionerKey)
-				if err != nil {
-					return xerrors.Errorf("validate provisioner key: %w", err)
-				}
-			}
-
 			logOpts := []clilog.Option{
 				clilog.WithFilter(logFilter...),
 				clilog.WithHuman(logHuman),
@@ -232,7 +216,7 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
 					},
 					Tags:           tags,
 					PreSharedKey:   preSharedKey,
-					Organization:   org.ID,
+					Organization:   orgID,
 					ProvisionerKey: provisionerKey,
 				})
 			}, &provisionerd.Options{
@@ -281,6 +265,13 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
 		},
 	}
 
+	keyOption := serpent.Option{
+		Flag:        "key",
+		Env:         "CODER_PROVISIONER_DAEMON_KEY",
+		Description: "Provisioner key to authenticate with Coder server.",
+		Value:       serpent.StringOf(&provisionerKey),
+		Hidden:      true,
+	}
 	cmd.Options = serpent.OptionSet{
 		{
 			Flag:          "cache-dir",
@@ -316,14 +307,9 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
 			Env:         "CODER_PROVISIONER_DAEMON_PSK",
 			Description: "Pre-shared key to authenticate with Coder server.",
 			Value:       serpent.StringOf(&preSharedKey),
+			UseInstead:  []serpent.Option{keyOption},
 		},
-		{
-			Flag:        "key",
-			Env:         "CODER_PROVISIONER_DAEMON_KEY",
-			Description: "Provisioner key to authenticate with Coder server.",
-			Value:       serpent.StringOf(&provisionerKey),
-			Hidden:      true,
-		},
+		keyOption,
 		{
 			Flag:        "name",
 			Env:         "CODER_PROVISIONER_DAEMON_NAME",

enterprise/cli/provisionerdaemonstart_test.go

@@ -68,46 +68,6 @@ func TestProvisionerDaemon_PSK(t *testing.T) {
 		require.Equal(t, proto.CurrentVersion.String(), daemons[0].APIVersion)
 	})
 
-	t.Run("AnotherOrg", func(t *testing.T) {
-		t.Parallel()
-		dv := coderdtest.DeploymentValues(t)
-		dv.Experiments = []string{string(codersdk.ExperimentMultiOrganization)}
-		client, _ := coderdenttest.New(t, &coderdenttest.Options{
-			Options: &coderdtest.Options{
-				DeploymentValues: dv,
-			},
-			ProvisionerDaemonPSK: "provisionersftw",
-			LicenseOptions: &coderdenttest.LicenseOptions{
-				Features: license.Features{
-					codersdk.FeatureExternalProvisionerDaemons: 1,
-					codersdk.FeatureMultipleOrganizations:      1,
-				},
-			},
-		})
-		anotherOrg := coderdenttest.CreateOrganization(t, client, coderdenttest.CreateOrganizationOptions{})
-		inv, conf := newCLI(t, "provisionerd", "start", "--psk=provisionersftw", "--name", "org-daemon", "--org", anotherOrg.ID.String())
-		err := conf.URL().Write(client.URL.String())
-		require.NoError(t, err)
-		pty := ptytest.New(t).Attach(inv)
-		ctx, cancel := context.WithTimeout(inv.Context(), testutil.WaitLong)
-		defer cancel()
-		clitest.Start(t, inv)
-		pty.ExpectMatchContext(ctx, "starting provisioner daemon")
-
-		var daemons []codersdk.ProvisionerDaemon
-		require.Eventually(t, func() bool {
-			daemons, err = client.OrganizationProvisionerDaemons(ctx, anotherOrg.ID)
-			if err != nil {
-				return false
-			}
-			return len(daemons) == 1
-		}, testutil.WaitShort, testutil.IntervalSlow)
-		assert.Equal(t, "org-daemon", daemons[0].Name)
-		assert.Equal(t, provisionersdk.ScopeOrganization, daemons[0].Tags[provisionersdk.TagScope])
-		assert.Equal(t, buildinfo.Version(), daemons[0].Version)
-		assert.Equal(t, proto.CurrentVersion.String(), daemons[0].APIVersion)
-	})
-
 	t.Run("AnotherOrgByNameWithUser", func(t *testing.T) {
 		t.Parallel()
 		dv := coderdtest.DeploymentValues(t)
@@ -126,7 +86,7 @@ func TestProvisionerDaemon_PSK(t *testing.T) {
 		})
 		anotherOrg := coderdenttest.CreateOrganization(t, client, coderdenttest.CreateOrganizationOptions{})
 		anotherClient, _ := coderdtest.CreateAnotherUser(t, client, anotherOrg.ID, rbac.RoleTemplateAdmin())
-		inv, conf := newCLI(t, "provisionerd", "start", "--psk=provisionersftw", "--name", "org-daemon", "--org", anotherOrg.Name)
+		inv, conf := newCLI(t, "provisionerd", "start", "--name", "org-daemon", "--org", anotherOrg.Name)
 		clitest.SetupConfig(t, anotherClient, conf)
 		pty := ptytest.New(t).Attach(inv)
 		ctx, cancel := context.WithTimeout(inv.Context(), testutil.WaitLong)
@@ -135,32 +95,6 @@ func TestProvisionerDaemon_PSK(t *testing.T) {
 		pty.ExpectMatchContext(ctx, "starting provisioner daemon")
 	})
 
-	t.Run("AnotherOrgByNameNoUser", func(t *testing.T) {
-		t.Parallel()
-		dv := coderdtest.DeploymentValues(t)
-		dv.Experiments = []string{string(codersdk.ExperimentMultiOrganization)}
-		client, _ := coderdenttest.New(t, &coderdenttest.Options{
-			Options: &coderdtest.Options{
-				DeploymentValues: dv,
-			},
-			ProvisionerDaemonPSK: "provisionersftw",
-			LicenseOptions: &coderdenttest.LicenseOptions{
-				Features: license.Features{
-					codersdk.FeatureExternalProvisionerDaemons: 1,
-					codersdk.FeatureMultipleOrganizations:      1,
-				},
-			},
-		})
-		anotherOrg := coderdenttest.CreateOrganization(t, client, coderdenttest.CreateOrganizationOptions{})
-		inv, conf := newCLI(t, "provisionerd", "start", "--psk=provisionersftw", "--name", "org-daemon", "--org", anotherOrg.Name)
-		err := conf.URL().Write(client.URL.String())
-		require.NoError(t, err)
-		ctx, cancel := context.WithTimeout(inv.Context(), testutil.WaitLong)
-		defer cancel()
-		err = inv.WithContext(ctx).Run()
-		require.ErrorContains(t, err, "must provide an org ID when not authenticated as a user and organization is specified")
-	})
-
 	t.Run("NoUserNoPSK", func(t *testing.T) {
 		t.Parallel()
 		client, _ := coderdenttest.New(t, &coderdenttest.Options{
@@ -467,7 +401,7 @@ func TestProvisionerDaemon_ProvisionerKey(t *testing.T) {
 			Name: "dont-TEST-me",
 		})
 		require.NoError(t, err)
-		inv, conf := newCLI(t, "provisionerd", "start", "--org", anotherOrg.ID.String(), "--key", res.Key, "--name=matt-daemon")
+		inv, conf := newCLI(t, "provisionerd", "start", "--key", res.Key, "--name=matt-daemon")
 		err = conf.URL().Write(client.URL.String())
 		require.NoError(t, err)
 		pty := ptytest.New(t).Attach(inv)

enterprise/cli/testdata/coder_provisionerd_start_--help.golden

@@ -43,6 +43,7 @@ OPTIONS:
 
       --psk string, $CODER_PROVISIONER_DAEMON_PSK
           Pre-shared key to authenticate with Coder server.
+          DEPRECATED: Use --key instead.
 
   -t, --tag string-array, $CODER_PROVISIONERD_TAGS
           Tags to filter provisioner jobs by.