Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat(cli): allow specifying name of provisioner daemon#11077

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.

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

Already on GitHub?Sign in to your account

Merged
johnstcn merged 8 commits intomainfromcj/provisioner-daemon-name
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
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
40 changes: 40 additions & 0 deletionscli/cliutil/hostname.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
package cliutil

import (
"os"
"strings"
"sync"
)

var (
hostname string
hostnameOnce sync.Once
)

// Hostname returns the hostname of the machine, lowercased,
// with any trailing domain suffix stripped.
// It is cached after the first call.
// If the hostname cannot be determined, for any reason,
// localhost will be returned instead.
func Hostname() string {
hostnameOnce.Do(func() { hostname = getHostname() })
return hostname
}

func getHostname() string {
h, err := os.Hostname()
if err != nil {
// Something must be very wrong if this fails.
// We'll just return localhost and hope for the best.
return "localhost"
}

// On some platforms, the hostname can be an FQDN. We only want the hostname.
if idx := strings.Index(h, "."); idx != -1 {
h = h[:idx]
}

// For the sake of consistency, we also want to lowercase the hostname.
// Per RFC 4343, DNS lookups must be case-insensitive.
return strings.ToLower(h)
}
14 changes: 11 additions & 3 deletionscli/server.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -62,6 +62,7 @@ import (
"github.com/coder/coder/v2/buildinfo"
"github.com/coder/coder/v2/cli/clibase"
"github.com/coder/coder/v2/cli/cliui"
"github.com/coder/coder/v2/cli/cliutil"
"github.com/coder/coder/v2/cli/config"
"github.com/coder/coder/v2/coderd"
"github.com/coder/coder/v2/coderd/autobuild"
Expand All@@ -86,6 +87,7 @@ import (
"github.com/coder/coder/v2/coderd/unhanger"
"github.com/coder/coder/v2/coderd/updatecheck"
"github.com/coder/coder/v2/coderd/util/slice"
stringutil "github.com/coder/coder/v2/coderd/util/strings"
"github.com/coder/coder/v2/coderd/workspaceapps"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/cryptorand"
Expand DownExpand Up@@ -875,9 +877,14 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
defer provisionerdWaitGroup.Wait()
provisionerdMetrics := provisionerd.NewMetrics(options.PrometheusRegistry)
for i := int64(0); i < vals.Provisioner.Daemons.Value(); i++ {
suffix := fmt.Sprintf("%d", i)
// The suffix is added to the hostname, so we may need to trim to fit into
// the 64 character limit.
hostname := stringutil.Truncate(cliutil.Hostname(), 63-len(suffix))
name := fmt.Sprintf("%s-%s", hostname, suffix)
Copy link
Member

Choose a reason for hiding this comment

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

A hostname could include e.g._,/,--, etc. I believe validation would fail in this case.. should we sanitize (strip/replace/allowlist)?

Copy link
MemberAuthor

@johnstcnjohnstcnDec 7, 2023•
edited
Loading

Choose a reason for hiding this comment

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

Itshould not, but it turns out that you can write whatever you want to/proc/sys/kernel/hostname. Thehostname command will validate its input, but it looks likedocker will happily accept whatever you give it.

If we trim, we run the risk of collisions between machines namedfoo/bar,foo~bar, andfoo!"£$%^&*()_+{}~@:?,bar. I'm leaning towards not sanitizing this.

daemonCacheDir := filepath.Join(cacheDir, fmt.Sprintf("provisioner-%d", i))
daemon, err := newProvisionerDaemon(
ctx, coderAPI, provisionerdMetrics, logger, vals, daemonCacheDir, errCh, &provisionerdWaitGroup,
ctx, coderAPI, provisionerdMetrics, logger, vals, daemonCacheDir, errCh, &provisionerdWaitGroup, name,
)
if err != nil {
return xerrors.Errorf("create provisioner daemon: %w", err)
Expand DownExpand Up@@ -1243,6 +1250,7 @@ func newProvisionerDaemon(
cacheDir string,
errCh chan error,
wg *sync.WaitGroup,
name string,
) (srv *provisionerd.Server, err error) {
ctx, cancel := context.WithCancel(ctx)
defer func() {
Expand DownExpand Up@@ -1334,9 +1342,9 @@ func newProvisionerDaemon(
return provisionerd.New(func(ctx context.Context) (proto.DRPCProvisionerDaemonClient, error) {
// This debounces calls to listen every second. Read the comment
// in provisionerdserver.go to learn more!
return coderAPI.CreateInMemoryProvisionerDaemon(ctx)
return coderAPI.CreateInMemoryProvisionerDaemon(ctx, name)
}, &provisionerd.Options{
Logger: logger.Named("provisionerd"),
Logger: logger.Named(fmt.Sprintf("provisionerd-%s", name)),
UpdateInterval: time.Second,
ForceCancelInterval: cfg.Provisioner.ForceCancelInterval.Value(),
Connector: connector,
Expand Down
6 changes: 2 additions & 4 deletionscoderd/coderd.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,7 +21,6 @@ import (
"github.com/go-chi/chi/v5/middleware"
"github.com/google/uuid"
"github.com/klauspost/compress/zstd"
"github.com/moby/moby/pkg/namesgenerator"
"github.com/prometheus/client_golang/prometheus"
httpSwagger "github.com/swaggo/http-swagger/v2"
"go.opentelemetry.io/otel/trace"
Expand DownExpand Up@@ -1150,7 +1149,7 @@ func compressHandler(h http.Handler) http.Handler {

// CreateInMemoryProvisionerDaemon is an in-memory connection to a provisionerd.
// Useful when starting coderd and provisionerd in the same process.
func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context) (client proto.DRPCProvisionerDaemonClient, err error) {
func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context, name string) (client proto.DRPCProvisionerDaemonClient, err error) {
tracer := api.TracerProvider.Tracer(tracing.TracerName)
clientSession, serverSession := provisionersdk.MemTransportPipe()
defer func() {
Expand All@@ -1165,9 +1164,8 @@ func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context) (client pro
}

mux := drpcmux.New()
name := namesgenerator.GetRandomName(1)
api.Logger.Info(ctx, "starting in-memory provisioner daemon", slog.F("name", name))
logger := api.Logger.Named(fmt.Sprintf("inmem-provisionerd-%s", name))
logger.Info(ctx, "starting in-memory provisioner daemon")
srv, err := provisionerdserver.NewServer(
api.ctx,
api.AccessURL,
Expand Down
4 changes: 3 additions & 1 deletioncoderd/coderdtest/coderdtest.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -530,7 +530,7 @@ func NewProvisionerDaemon(t testing.TB, coderAPI *coderd.API) io.Closer {
}()

daemon := provisionerd.New(func(ctx context.Context) (provisionerdproto.DRPCProvisionerDaemonClient, error) {
return coderAPI.CreateInMemoryProvisionerDaemon(ctx)
return coderAPI.CreateInMemoryProvisionerDaemon(ctx, t.Name())
}, &provisionerd.Options{
Logger: coderAPI.Logger.Named("provisionerd").Leveled(slog.LevelDebug),
UpdateInterval: 250 * time.Millisecond,
Expand DownExpand Up@@ -567,6 +567,8 @@ func NewExternalProvisionerDaemon(t testing.TB, client *codersdk.Client, org uui

daemon := provisionerd.New(func(ctx context.Context) (provisionerdproto.DRPCProvisionerDaemonClient, error) {
return client.ServeProvisionerDaemon(ctx, codersdk.ServeProvisionerDaemonRequest{
ID: uuid.New(),
Name: t.Name(),
Organization: org,
Provisioners: []codersdk.ProvisionerType{codersdk.ProvisionerTypeEcho},
Tags: tags,
Expand Down
11 changes: 11 additions & 0 deletionscoderd/util/strings/strings.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,3 +17,14 @@ func JoinWithConjunction(s []string) string {
s[last],
)
}

// Truncate returns the first n characters of s.
func Truncate(s string, n int) string {
if n < 1 {
return ""
}
if len(s) <= n {
return s
}
return s[:n]
}
24 changes: 24 additions & 0 deletionscoderd/util/strings/strings_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,3 +14,27 @@ func TestJoinWithConjunction(t *testing.T) {
require.Equal(t, "foo and bar", strings.JoinWithConjunction([]string{"foo", "bar"}))
require.Equal(t, "foo, bar and baz", strings.JoinWithConjunction([]string{"foo", "bar", "baz"}))
}

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

for _, tt := range []struct {
s string
n int
expected string
}{
{"foo", 4, "foo"},
{"foo", 3, "foo"},
{"foo", 2, "fo"},
{"foo", 1, "f"},
{"foo", 0, ""},
{"foo", -1, ""},
} {
tt := tt
t.Run(tt.expected, func(t *testing.T) {
t.Parallel()
actual := strings.Truncate(tt.s, tt.n)
require.Equal(t, tt.expected, actual)
})
}
}
3 changes: 3 additions & 0 deletionscodersdk/provisionerdaemons.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -177,6 +177,8 @@ func (c *Client) provisionerJobLogsAfter(ctx context.Context, path string, after
type ServeProvisionerDaemonRequest struct {
// ID is a unique ID for a provisioner daemon.
ID uuid.UUID `json:"id" format:"uuid"`
// Name is the human-readable unique identifier for the daemon.
Name string `json:"name" example:"my-cool-provisioner-daemon"`
// Organization is the organization for the URL. At present provisioner daemons ARE NOT scoped to organizations
// and so the organization ID is optional.
Organization uuid.UUID `json:"organization" format:"uuid"`
Expand All@@ -198,6 +200,7 @@ func (c *Client) ServeProvisionerDaemon(ctx context.Context, req ServeProvisione
}
query := serverURL.Query()
query.Add("id", req.ID.String())
query.Add("name", req.Name)
Copy link
Member

Choose a reason for hiding this comment

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

curious: is it required to pass ID and name now?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I think I'm going to end up ignoring the ID parameter and just upserting based on name in a follow-up PR.

mtojek reacted with thumbs up emoji
for _, provisioner := range req.Provisioners {
query.Add("provisioner", string(provisioner))
}
Expand Down
9 changes: 9 additions & 0 deletionsdocs/cli/provisionerd_start.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

33 changes: 31 additions & 2 deletionsenterprise/cli/provisionerdaemons.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@ import (
"context"
"fmt"
"os"
"regexp"
"time"

"github.com/google/uuid"
Expand All@@ -16,6 +17,7 @@ import (
agpl "github.com/coder/coder/v2/cli"
"github.com/coder/coder/v2/cli/clibase"
"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/provisionerdserver"
"github.com/coder/coder/v2/codersdk"
Expand All@@ -41,13 +43,24 @@ func (r *RootCmd) provisionerDaemons() *clibase.Cmd {
return cmd
}

func validateProvisionerDaemonName(name string) error {
if len(name) > 64 {
return xerrors.Errorf("name cannot be greater than 64 characters in length")
}
if ok, err := regexp.MatchString(`^[a-zA-Z0-9][a-zA-Z0-9-]{0,61}[a-zA-Z0-9]$`, name); err != nil || !ok {
return xerrors.Errorf("name %q is not a valid hostname", name)
}
return nil
}

func (r *RootCmd) provisionerDaemonStart() *clibase.Cmd {
var (
cacheDir string
rawTags []string
pollInterval time.Duration
pollJitter time.Duration
preSharedKey string
name string
)
client := new(codersdk.Client)
cmd := &clibase.Cmd{
Expand All@@ -68,6 +81,14 @@ func (r *RootCmd) provisionerDaemonStart() *clibase.Cmd {
return err
}

if name == "" {
name = cliutil.Hostname()
}

if err := validateProvisionerDaemonName(name); err != nil {
return err
}

logger := slog.Make(sloghuman.Sink(inv.Stderr))
if ok, _ := inv.ParsedFlags().GetBool("verbose"); ok {
logger = logger.Leveled(slog.LevelDebug)
Expand DownExpand Up@@ -122,15 +143,16 @@ func (r *RootCmd) provisionerDaemonStart() *clibase.Cmd {
}
}()

logger.Info(ctx, "starting provisioner daemon", slog.F("tags", tags))
logger.Info(ctx, "starting provisioner daemon", slog.F("tags", tags), slog.F("name", name))

connector := provisionerd.LocalProvisioners{
string(database.ProvisionerTypeTerraform): proto.NewDRPCProvisionerClient(terraformClient),
}
id := uuid.New()
srv := provisionerd.New(func(ctx context.Context) (provisionerdproto.DRPCProvisionerDaemonClient, error) {
return client.ServeProvisionerDaemon(ctx, codersdk.ServeProvisionerDaemonRequest{
ID: id,
ID: id,
Name: name,
Provisioners: []codersdk.ProvisionerType{
codersdk.ProvisionerTypeTerraform,
},
Expand DownExpand Up@@ -205,6 +227,13 @@ func (r *RootCmd) provisionerDaemonStart() *clibase.Cmd {
Description: "Pre-shared key to authenticate with Coder server.",
Value: clibase.StringOf(&preSharedKey),
},
{
Flag: "name",
Env: "CODER_PROVISIONER_DAEMON_NAME",
Description: "Name of this provisioner daemon. Defaults to the current hostname without FQDN.",
Value: clibase.StringOf(&name),
Default: "",
},
}

return cmd
Expand Down
3 changes: 2 additions & 1 deletionenterprise/cli/provisionerdaemons_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,14 +26,15 @@ func TestProvisionerDaemon_PSK(t *testing.T) {
},
},
})
inv, conf := newCLI(t, "provisionerd", "start", "--psk=provisionersftw")
inv, conf := newCLI(t, "provisionerd", "start", "--psk=provisionersftw", "--name=matt-daemon")
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")
pty.ExpectMatchContext(ctx, "matt-daemon")
}

func TestProvisionerDaemon_SessionToken(t *testing.T) {
Expand Down
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,10 @@ OPTIONS:
-c, --cache-dir string, $CODER_CACHE_DIRECTORY (default: [cache dir])
Directory to store cached data.

--name string, $CODER_PROVISIONER_DAEMON_NAME
Name of this provisioner daemon. Defaults to the current hostname
without FQDN.

--poll-interval duration, $CODER_PROVISIONERD_POLL_INTERVAL (default: 1s)
Deprecated and ignored.

Expand Down
8 changes: 7 additions & 1 deletionenterprise/coderd/provisionerdaemons.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -178,6 +178,13 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
}
}

name := namesgenerator.GetRandomName(10)
if vals, ok := r.URL.Query()["name"]; ok && len(vals) > 0 {
name = vals[0]
} else {
api.Logger.Warn(ctx, "unnamed provisioner daemon")
}

tags, authorized := api.provisionerDaemonAuth.authorize(r, tags)
if !authorized {
api.Logger.Warn(ctx, "unauthorized provisioner daemon serve request", slog.F("tags", tags))
Expand DownExpand Up@@ -206,7 +213,6 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
}
}

name := namesgenerator.GetRandomName(1)
log := api.Logger.With(
slog.F("name", name),
slog.F("provisioners", provisioners),
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp