Skip to content

feat: add --key flag to provisionerd start #14002

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 25, 2024
Merged

feat: add --key flag to provisionerd start #14002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f4f1b1b
feat: add --key flag to provisionerd start
f0ssel Jul 24, 2024
47aa9c4
tests
f0ssel Jul 25, 2024
aa314a3
fix test
f0ssel Jul 25, 2024
7b83b7e
more tests
f0ssel Jul 25, 2024
727abcd
restrict route
f0ssel Jul 25, 2024
e54e662
fix tags with key flag
f0ssel Jul 25, 2024
10b09ed
fix tags with key flag
f0ssel Jul 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 36 additions & 8 deletions enterprise/cli/provisionerdaemonstart.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ 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"
Expand All @@ -46,6 +47,7 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
pollInterval time.Duration
pollJitter time.Duration
preSharedKey string
provisionerKey string
verbose bool

prometheusEnable bool
Expand Down Expand Up @@ -83,8 +85,8 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
return xerrors.Errorf("current organization: %w", err)
}

if preSharedKey == "" {
return xerrors.New("must provide a pre-shared key when not authenticated as a user")
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}}
Expand Down Expand Up @@ -113,6 +115,19 @@ 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.Parse(provisionerKey)
if err != nil {
return xerrors.Errorf("parse provisioner key: %w", err)
}
}

logOpts := []clilog.Option{
clilog.WithFilter(logFilter...),
clilog.WithHuman(logHuman),
Expand All @@ -136,12 +151,17 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
logger.Info(ctx, "note: untagged provisioners can only pick up jobs from untagged templates")
}

// When authorizing with a PSK, we automatically scope the provisionerd
// to organization. Scoping to user with PSK auth is not a valid configuration.
// When authorizing with a PSK / provisioner key, we automatically scope the provisionerd
// to organization. Scoping to user with PSK / provisioner key auth is not a valid configuration.
if preSharedKey != "" {
logger.Info(ctx, "psk auth automatically sets tag "+provisionersdk.TagScope+"="+provisionersdk.ScopeOrganization)
logger.Info(ctx, "psk automatically sets tag "+provisionersdk.TagScope+"="+provisionersdk.ScopeOrganization)
tags[provisionersdk.TagScope] = provisionersdk.ScopeOrganization
}
if provisionerKey != "" {
logger.Info(ctx, "provisioner key auth automatically sets tag "+provisionersdk.TagScope+" empty")
// no scope tag will default to org scope
delete(tags, provisionersdk.TagScope)
}
Comment on lines +160 to +164
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this because it's all done on coderd? And we should be passing no tags into the provisioner daemon?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, the API will actually fail if you pass any tags + a provisioner key. I felt this was a good opportunity to break that behavior altogether.


err = os.MkdirAll(cacheDir, 0o700)
if err != nil {
Expand Down Expand Up @@ -210,9 +230,10 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
Provisioners: []codersdk.ProvisionerType{
codersdk.ProvisionerTypeTerraform,
},
Tags: tags,
PreSharedKey: preSharedKey,
Organization: org.ID,
Tags: tags,
PreSharedKey: preSharedKey,
Organization: org.ID,
ProvisionerKey: provisionerKey,
})
}, &provisionerd.Options{
Logger: logger,
Expand Down Expand Up @@ -296,6 +317,13 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
Description: "Pre-shared key to authenticate with Coder server.",
Value: serpent.StringOf(&preSharedKey),
},
{
Flag: "key",
Env: "CODER_PROVISIONER_DAEMON_KEY",
Description: "Provisioner key to authenticate with Coder server.",
Value: serpent.StringOf(&provisionerKey),
Hidden: true,
},
{
Flag: "name",
Env: "CODER_PROVISIONER_DAEMON_NAME",
Expand Down
161 changes: 160 additions & 1 deletion enterprise/cli/provisionerdaemonstart_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func TestProvisionerDaemon_PSK(t *testing.T) {
ctx, cancel := context.WithTimeout(inv.Context(), testutil.WaitLong)
defer cancel()
err = inv.WithContext(ctx).Run()
require.ErrorContains(t, err, "must provide a pre-shared key when not authenticated as a user")
require.ErrorContains(t, err, "must provide a pre-shared key or provisioner key when not authenticated as a user")
})
}

Expand Down Expand Up @@ -301,6 +301,165 @@ func TestProvisionerDaemon_SessionToken(t *testing.T) {
})
}

func TestProvisionerDaemon_ProvisionerKey(t *testing.T) {
t.Parallel()

t.Run("OK", func(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
dv := coderdtest.DeploymentValues(t)
dv.Experiments.Append(string(codersdk.ExperimentMultiOrganization))
client, user := coderdenttest.New(t, &coderdenttest.Options{
ProvisionerDaemonPSK: "provisionersftw",
LicenseOptions: &coderdenttest.LicenseOptions{
Features: license.Features{
codersdk.FeatureExternalProvisionerDaemons: 1,
codersdk.FeatureMultipleOrganizations: 1,
},
},
Options: &coderdtest.Options{
DeploymentValues: dv,
},
})
// nolint:gocritic // test
res, err := client.CreateProvisionerKey(ctx, user.OrganizationID, codersdk.CreateProvisionerKeyRequest{
Name: "dont-TEST-me",
})
require.NoError(t, err)
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)
clitest.Start(t, inv)
pty.ExpectNoMatchBefore(ctx, "check entitlement", "starting provisioner daemon")
pty.ExpectMatchContext(ctx, "matt-daemon")

var daemons []codersdk.ProvisionerDaemon
require.Eventually(t, func() bool {
daemons, err = client.OrganizationProvisionerDaemons(ctx, user.OrganizationID)
if err != nil {
return false
}
return len(daemons) == 1
}, testutil.WaitShort, testutil.IntervalSlow)
require.Equal(t, "matt-daemon", daemons[0].Name)
require.Equal(t, provisionersdk.ScopeOrganization, daemons[0].Tags[provisionersdk.TagScope])
require.Equal(t, buildinfo.Version(), daemons[0].Version)
require.Equal(t, proto.CurrentVersion.String(), daemons[0].APIVersion)
})

t.Run("NoPSK", func(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
dv := coderdtest.DeploymentValues(t)
dv.Experiments.Append(string(codersdk.ExperimentMultiOrganization))
client, user := coderdenttest.New(t, &coderdenttest.Options{
ProvisionerDaemonPSK: "provisionersftw",
LicenseOptions: &coderdenttest.LicenseOptions{
Features: license.Features{
codersdk.FeatureExternalProvisionerDaemons: 1,
codersdk.FeatureMultipleOrganizations: 1,
},
},
Options: &coderdtest.Options{
DeploymentValues: dv,
},
})
// nolint:gocritic // test
res, err := client.CreateProvisionerKey(ctx, user.OrganizationID, codersdk.CreateProvisionerKeyRequest{
Name: "dont-TEST-me",
})
require.NoError(t, err)
inv, conf := newCLI(t, "provisionerd", "start", "--psk", "provisionersftw", "--key", res.Key, "--name=matt-daemon")
err = conf.URL().Write(client.URL.String())
require.NoError(t, err)
err = inv.WithContext(ctx).Run()
require.ErrorContains(t, err, "cannot provide both provisioner key --key and pre-shared key --psk")
})

t.Run("NoTags", func(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
dv := coderdtest.DeploymentValues(t)
dv.Experiments.Append(string(codersdk.ExperimentMultiOrganization))
client, user := coderdenttest.New(t, &coderdenttest.Options{
ProvisionerDaemonPSK: "provisionersftw",
LicenseOptions: &coderdenttest.LicenseOptions{
Features: license.Features{
codersdk.FeatureExternalProvisionerDaemons: 1,
codersdk.FeatureMultipleOrganizations: 1,
},
},
Options: &coderdtest.Options{
DeploymentValues: dv,
},
})
// nolint:gocritic // test
res, err := client.CreateProvisionerKey(ctx, user.OrganizationID, codersdk.CreateProvisionerKeyRequest{
Name: "dont-TEST-me",
})
require.NoError(t, err)
inv, conf := newCLI(t, "provisionerd", "start", "--tag", "mykey=yourvalue", "--key", res.Key, "--name=matt-daemon")
err = conf.URL().Write(client.URL.String())
require.NoError(t, err)
err = inv.WithContext(ctx).Run()
require.ErrorContains(t, err, "cannot provide tags when using provisioner key")
})

t.Run("AnotherOrg", func(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
dv := coderdtest.DeploymentValues(t)
dv.Experiments.Append(string(codersdk.ExperimentMultiOrganization))
client, _ := coderdenttest.New(t, &coderdenttest.Options{
ProvisionerDaemonPSK: "provisionersftw",
LicenseOptions: &coderdenttest.LicenseOptions{
Features: license.Features{
codersdk.FeatureExternalProvisionerDaemons: 1,
codersdk.FeatureMultipleOrganizations: 1,
},
},
Options: &coderdtest.Options{
DeploymentValues: dv,
},
})
anotherOrg := coderdtest.CreateOrganization(t, client, coderdtest.CreateOrganizationOptions{})
// nolint:gocritic // test
res, err := client.CreateProvisionerKey(ctx, anotherOrg.ID, codersdk.CreateProvisionerKeyRequest{
Name: "dont-TEST-me",
})
require.NoError(t, err)
inv, conf := newCLI(t, "provisionerd", "start", "--org", anotherOrg.ID.String(), "--key", res.Key, "--name=matt-daemon")
err = conf.URL().Write(client.URL.String())
require.NoError(t, err)
pty := ptytest.New(t).Attach(inv)
clitest.Start(t, inv)
pty.ExpectNoMatchBefore(ctx, "check entitlement", "starting provisioner daemon")
pty.ExpectMatchContext(ctx, "matt-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)
require.Equal(t, "matt-daemon", daemons[0].Name)
require.Equal(t, provisionersdk.ScopeOrganization, daemons[0].Tags[provisionersdk.TagScope])
require.Equal(t, buildinfo.Version(), daemons[0].Version)
require.Equal(t, proto.CurrentVersion.String(), daemons[0].APIVersion)
})
}

//nolint:paralleltest,tparallel // Test uses a static port.
func TestProvisionerDaemon_PrometheusEnabled(t *testing.T) {
// Ephemeral ports have a tendency to conflict and fail with `bind: address already in use` error.
Expand Down
53 changes: 29 additions & 24 deletions enterprise/coderd/provisionerdaemons.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/hashicorp/yamux"
"github.com/moby/moby/pkg/namesgenerator"
"go.opentelemetry.io/otel/trace"
"golang.org/x/exp/maps"
"golang.org/x/xerrors"
"nhooyr.io/websocket"
"storj.io/drpc/drpcmux"
Expand Down Expand Up @@ -97,39 +98,43 @@ func (p *provisionerDaemonAuth) authorize(r *http.Request, orgID uuid.UUID, tags
return nil, xerrors.New("Both API key and provisioner key authentication provided. Only one is allowed.")
}

if apiKeyOK {
tags = provisionersdk.MutateTags(apiKey.UserID, tags)
if tags[provisionersdk.TagScope] == provisionersdk.ScopeUser {
// Any authenticated user can create provisioner daemons scoped
// for jobs that they own,
return tags, nil
// Provisioner Key Auth
if pkOK {
if pk.OrganizationID != orgID {
return nil, xerrors.New("provisioner key unauthorized")
}
ua := httpmw.UserAuthorization(r)
err := p.authorizer.Authorize(ctx, ua, policy.ActionCreate, rbac.ResourceProvisionerDaemon.InOrg(orgID))
if err != nil {
if !provAuth {
return nil, xerrors.New("user unauthorized")
}

// Allow fallback to PSK auth if the user is not allowed to create provisioner daemons.
// This is to preserve backwards compatibility with existing user provisioner daemons.
// If using PSK auth, the daemon is, by definition, scoped to the organization.
tags = provisionersdk.MutateTags(uuid.Nil, tags)
return tags, nil
if tags != nil && !maps.Equal(tags, map[string]string{}) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL maps.Equal

return nil, xerrors.New("tags are not allowed when using a provisioner key")
}

// User is allowed to create provisioner daemons
// If using provisioner key / PSK auth, the daemon is, by definition, scoped to the organization.
// Use the provisioner key tags here.
tags = provisionersdk.MutateTags(uuid.Nil, pk.Tags)
return tags, nil
}

if pkOK {
if pk.OrganizationID != orgID {
return nil, xerrors.New("provisioner key unauthorized")
// User Auth
tags = provisionersdk.MutateTags(apiKey.UserID, tags)
if tags[provisionersdk.TagScope] == provisionersdk.ScopeUser {
// Any authenticated user can create provisioner daemons scoped
// for jobs that they own,
return tags, nil
}
ua := httpmw.UserAuthorization(r)
err := p.authorizer.Authorize(ctx, ua, policy.ActionCreate, rbac.ResourceProvisionerDaemon.InOrg(orgID))
if err != nil {
if !provAuth {
return nil, xerrors.New("user unauthorized")
}

// Allow fallback to PSK auth if the user is not allowed to create provisioner daemons.
// This is to preserve backwards compatibility with existing user provisioner daemons.
// If using PSK auth, the daemon is, by definition, scoped to the organization.
tags = provisionersdk.MutateTags(uuid.Nil, tags)
return tags, nil
}

// If using provisioner key / PSK auth, the daemon is, by definition, scoped to the organization.
tags = provisionersdk.MutateTags(uuid.Nil, tags)
// User is allowed to create provisioner daemons
return tags, nil
}

Expand Down
3 changes: 0 additions & 3 deletions enterprise/coderd/provisionerdaemons_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,9 +703,6 @@ func TestProvisionerDaemonServe(t *testing.T) {
Provisioners: []codersdk.ProvisionerType{
codersdk.ProvisionerTypeEcho,
},
Tags: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
},
PreSharedKey: tc.requestPSK,
ProvisionerKey: tc.requestProvisionerKey,
})
Expand Down
Loading