diff --git a/commands/apps_dev.go b/commands/apps_dev.go index 91ce88fb69..fc99b265d0 100644 --- a/commands/apps_dev.go +++ b/commands/apps_dev.go @@ -112,10 +112,13 @@ func RunAppsDevBuild(c *CmdConfig) error { conf, err := newAppDevConfig(c) if err != nil { - return err + return fmt.Errorf("initializing config: %w", err) } - var spec *godo.AppSpec + var ( + spec *godo.AppSpec + cnbVersioning *godo.AppBuildConfigCNBVersioning + ) appID, err := conf.GetString(doctl.ArgApp) if err != nil { return err @@ -124,11 +127,13 @@ func RunAppsDevBuild(c *CmdConfig) error { // TODO: if this is the user's first time running dev build, ask them if they'd like to // link an existing app. if appID != "" { + template.Print(`{{success checkmark}} fetching app details{{nl}}`, AppsDevDefaultSpecPath) app, err := c.Apps().Get(appID) if err != nil { return err } spec = app.Spec + cnbVersioning = app.GetBuildConfig().GetCNBVersioning() } appSpecPath, err := conf.GetString(doctl.ArgAppSpec) @@ -136,20 +141,16 @@ func RunAppsDevBuild(c *CmdConfig) error { return err } - if spec == nil { - if appSpecPath == "" { - if _, err := os.Stat(AppsDevDefaultSpecPath); err == nil { - appSpecPath = AppsDevDefaultSpecPath - template.Print(heredoc.Doc(` - {{success checkmark}} using app spec at {{highlight .}}{{nl}}`, - ), AppsDevDefaultSpecPath) - } + if spec == nil && appSpecPath == "" { + if _, err := os.Stat(AppsDevDefaultSpecPath); err == nil { + appSpecPath = AppsDevDefaultSpecPath + template.Print(`{{success checkmark}} using app spec at {{highlight .}}{{nl}}`, AppsDevDefaultSpecPath) } - if appSpecPath != "" { - spec, err = readAppSpec(os.Stdin, appSpecPath) - if err != nil { - return err - } + } + if appSpecPath != "" { + spec, err = readAppSpec(os.Stdin, appSpecPath) + if err != nil { + return err } } @@ -171,26 +172,31 @@ func RunAppsDevBuild(c *CmdConfig) error { if len(c.Args) >= 1 { component = c.Args[0] } - if Interactive && component == "" { + if component == "" { var components []list.Item _ = godo.ForEachAppSpecComponent(spec, func(c godo.AppBuildableComponentSpec) error { components = append(components, componentListItem{c}) return nil }) - list := list.New(components) - list.Model().Title = "select a component" - list.Model().SetStatusBarItemName("component", "components") - selected, err := list.Select() - if err != nil { - return err - } else if selected == nil { - return fmt.Errorf("cancelled") - } - selectedComponent, ok := selected.(componentListItem) - if !ok { - return fmt.Errorf("unexpected item type %T", selectedComponent) + + if len(components) == 1 { + component = components[0].(componentListItem).spec.GetName() + } else if len(components) > 1 && Interactive { + list := list.New(components) + list.Model().Title = "select a component" + list.Model().SetStatusBarItemName("component", "components") + selected, err := list.Select() + if err != nil { + return err + } else if selected == nil { + return fmt.Errorf("cancelled") + } + selectedComponent, ok := selected.(componentListItem) + if !ok { + return fmt.Errorf("unexpected item type %T", selectedComponent) + } + component = selectedComponent.spec.GetName() } - component = selectedComponent.spec.GetName() } if component == "" { @@ -205,9 +211,10 @@ func RunAppsDevBuild(c *CmdConfig) error { return err } - buildingComponentLine := template.String(heredoc.Doc(` - building {{lower (snakeToTitle .GetType)}} {{highlight .GetName}}`, - ), componentSpec) + buildingComponentLine := template.String( + `building {{lower (snakeToTitle .GetType)}} {{highlight .GetName}}`, + componentSpec, + ) template.Print(`{{success checkmark}} {{.}}{{nl 2}}`, buildingComponentLine) if componentSpec.GetSourceDir() != "" { @@ -240,9 +247,6 @@ func RunAppsDevBuild(c *CmdConfig) error { if err != nil { return err } - if registryName == "" { - return errors.New("registry-name is required") - } buildOverrride, err := conf.GetString(doctl.ArgAppDevBuildCommand) if err != nil { @@ -306,6 +310,7 @@ func RunAppsDevBuild(c *CmdConfig) error { EnvOverride: envs, BuildCommandOverride: buildOverrride, LogWriter: logWriter, + Versioning: builder.Versioning{CNB: cnbVersioning}, }) if err != nil { return err diff --git a/commands/apps_dev_config.go b/commands/apps_dev_config.go index f75414adbd..325002bd71 100644 --- a/commands/apps_dev_config.go +++ b/commands/apps_dev_config.go @@ -94,7 +94,7 @@ func RunAppsDevConfigSet(c *CmdConfig) error { } for _, arg := range c.Args { - split := strings.Split(arg, "=") + split := strings.SplitN(arg, "=", 2) if len(split) != 2 { return errors.New("unexpected arg: " + arg) } @@ -223,7 +223,7 @@ func newAppDevConfig(cmdConfig *CmdConfig) (*appDevConfig, error) { if err := ensureStringInFile(devConfigFilePath, ""); err != nil { return nil, err } - if err := ensureStringInFile(filepath.Join(configDir, ".gitignore"), "dev-config.yaml"); err != nil { + if err := ensureStringInFile(filepath.Join(configDir, ".gitignore"), DefaultDevConfigFile); err != nil { return nil, err } } else if _, err := os.Stat(devConfigFilePath); err != nil { diff --git a/commands/charm/pager/pager.go b/commands/charm/pager/pager.go index 8837adc00f..01d086f2b5 100644 --- a/commands/charm/pager/pager.go +++ b/commands/charm/pager/pager.go @@ -60,7 +60,9 @@ func WithTitle(title string) Option { func (p *Pager) Write(b []byte) (int, error) { n, err := p.buffer.Write(b) - p.prog.Send(msgUpdate{}) + if p.prog != nil { + p.prog.Send(msgUpdate{}) + } return n, err } diff --git a/commands/charm/util.go b/commands/charm/util.go index fc10e5e9c2..e41ceea25c 100644 --- a/commands/charm/util.go +++ b/commands/charm/util.go @@ -18,6 +18,10 @@ func Indent(level uint) io.Writer { return indent.NewWriterPipe(os.Stdout, level, nil) } +func IndentWriter(w io.Writer, level uint) io.Writer { + return indent.NewWriterPipe(w, level, nil) +} + func IndentString(level uint, str string) string { return indent.String(str, level) } diff --git a/internal/apps/builder/builder.go b/internal/apps/builder/builder.go index 1078bfdf04..f7759c0467 100644 --- a/internal/apps/builder/builder.go +++ b/internal/apps/builder/builder.go @@ -10,7 +10,7 @@ import ( "os" "time" - "github.com/MakeNowJust/heredoc" + "github.com/digitalocean/doctl/commands/charm" "github.com/digitalocean/doctl/commands/charm/template" "github.com/digitalocean/godo" ) @@ -46,7 +46,12 @@ type baseComponentBuilder struct { } func (b baseComponentBuilder) ImageOutputName() string { - return fmt.Sprintf("%s/%s:dev", b.registry, b.component.GetName()) + ref := fmt.Sprintf("%s:dev", b.component.GetName()) + if b.registry != "" { + ref = fmt.Sprintf("%s/%s", b.registry, ref) + } + + return ref } func (b baseComponentBuilder) getLogWriter() io.Writer { @@ -56,55 +61,42 @@ func (b baseComponentBuilder) getLogWriter() io.Writer { return b.logWriter } -func (b baseComponentBuilder) getEnvMap() map[string]string { - envs := map[string]string{} +func (b baseComponentBuilder) getEnvMap() (map[string]string, error) { + envMap := map[string]string{} lw := b.getLogWriter() + template.Render(lw, `{{success checkmark}} configuring build environment variables{{nl}}`, nil) + subLW := charm.IndentWriter(lw, 2) - template.Render(lw, heredoc.Doc(` - {{success checkmark}} configuring build environment variables... {{nl 2}}`, - ), nil) - - if b.spec != nil { - for _, e := range b.spec.Envs { + addEnvs := func(envs ...*godo.AppVariableDefinition) { + for _, e := range envs { if e.Type == godo.AppVariableType_Secret { - template.Render(lw, heredoc.Doc(` - => Ignoring SECRET variable {{highlight .GetKey}}{{nl}}`, - ), e) + template.Render(subLW, `{{success checkmark}} ignoring SECRET variable {{highlight .GetKey}}{{nl}}`, e) continue } if e.Scope != godo.AppVariableScope_RunTime { val := e.Value - envs[e.Key] = val + envMap[e.Key] = val } } } - for _, e := range b.component.GetEnvs() { - if e.Type == godo.AppVariableType_Secret { - template.Render(lw, heredoc.Doc(` - => Ignoring SECRET variable {{highlight .GetKey}}{{nl}}`, - ), e) - continue - } - if e.Scope != godo.AppVariableScope_RunTime { - val := e.Value - envs[e.Key] = val - } - } + addEnvs(b.spec.GetEnvs()...) + addEnvs(b.component.GetEnvs()...) for k, v := range b.envOverrides { v := v - if _, ok := envs[k]; ok { - template.Render(lw, heredoc.Doc(` - => Overwriting {{highlight .}} with provided env value{{nl}}`, - ), k) + _, exists := envMap[k] + if !exists { + // TODO: if interactive prompt to auto add to spec + return nil, fmt.Errorf("variable not in found in app spec: %s", k) } - envs[k] = v + template.Render(subLW, `{{success checkmark}} overriding value for variable {{highlight .}}{{nl}}`, k) + envMap[k] = v } fmt.Fprint(lw, "\n") - return envs + return envMap, nil } // NewBuilderOpts ... @@ -114,6 +106,11 @@ type NewBuilderOpts struct { EnvOverride map[string]string BuildCommandOverride string LogWriter io.Writer + Versioning Versioning +} + +type Versioning struct { + CNB *godo.AppBuildConfigCNBVersioning } // DefaultComponentBuilderFactory is the standard component builder factory. @@ -141,6 +138,14 @@ func (f *DefaultComponentBuilderFactory) NewComponentBuilder(cli DockerEngineCli copyOnWriteSemantics := true if component.GetDockerfilePath() == "" { + var cnbVersioning CNBVersioning + for _, bp := range opts.Versioning.CNB.GetBuildpacks() { + cnbVersioning.Buildpacks = append(cnbVersioning.Buildpacks, &Buildpack{ + ID: bp.ID, + Version: fmt.Sprintf("%d.0.0", bp.MajorVersion), + }) + } + return &CNBComponentBuilder{ baseComponentBuilder: baseComponentBuilder{ cli, @@ -153,6 +158,7 @@ func (f *DefaultComponentBuilderFactory) NewComponentBuilder(cli DockerEngineCli copyOnWriteSemantics, opts.LogWriter, }, + versioning: cnbVersioning, }, nil } diff --git a/internal/apps/builder/builder_test.go b/internal/apps/builder/builder_test.go index 2b742c6506..dbde03fa41 100644 --- a/internal/apps/builder/builder_test.go +++ b/internal/apps/builder/builder_test.go @@ -34,7 +34,24 @@ func TestNewBuilderComponent(t *testing.T) { require.ErrorContains(t, err, fmt.Sprintf("component %s not found", missingComponent)) }) - t.Run("cnb builder", func(t *testing.T) { + t.Run("dockerfile builder", func(t *testing.T) { + builder, err := builderFactory.NewComponentBuilder(nil, ".", &godo.AppSpec{ + Services: []*godo.AppServiceSpec{{ + Name: "web", + DockerfilePath: ".", + }}, + }, NewBuilderOpts{ + Component: "web", + }) + require.NoError(t, err) + require.IsTypef(t, &DockerComponentBuilder{}, builder, "expected DockerComponentBuilder but was %T", builder) + }) +} + +func TestNewBuilderComponent_CNB(t *testing.T) { + builderFactory := DefaultComponentBuilderFactory{} + + t.Run("happy path", func(t *testing.T) { builder, err := builderFactory.NewComponentBuilder(nil, ".", &godo.AppSpec{ Services: []*godo.AppServiceSpec{{ Name: "web", @@ -44,18 +61,48 @@ func TestNewBuilderComponent(t *testing.T) { }) require.NoError(t, err) require.IsTypef(t, &CNBComponentBuilder{}, builder, "expected CNBComponentBuilder but was %T", builder) + + cnbBuilder := builder.(*CNBComponentBuilder) + // no buildpacks in builder opts + require.Equal(t, CNBVersioning{}, cnbBuilder.versioning) }) - t.Run("dockerfile builder", func(t *testing.T) { + t.Run("buildpack versioning", func(t *testing.T) { builder, err := builderFactory.NewComponentBuilder(nil, ".", &godo.AppSpec{ Services: []*godo.AppServiceSpec{{ - Name: "web", - DockerfilePath: ".", + Name: "web", }}, }, NewBuilderOpts{ Component: "web", + Versioning: Versioning{ + CNB: &godo.AppBuildConfigCNBVersioning{ + Buildpacks: []*godo.Buildpack{ + { + ID: "digitalocean/node", + MajorVersion: 1, + }, + { + ID: "digitalocean/go", + MajorVersion: 2, + }, + }, + }, + }, }) require.NoError(t, err) - require.IsTypef(t, &DockerComponentBuilder{}, builder, "expected DockerComponentBuilder but was %T", builder) + cnbBuilder, ok := builder.(*CNBComponentBuilder) + require.True(t, ok, "expected CNBComponentBuilder but was %T", builder) + require.Equal(t, CNBVersioning{ + Buildpacks: []*Buildpack{ + { + ID: "digitalocean/node", + Version: "1.0.0", + }, + { + ID: "digitalocean/go", + Version: "2.0.0", + }, + }, + }, cnbBuilder.versioning) }) } diff --git a/internal/apps/builder/cnb.go b/internal/apps/builder/cnb.go index ebde433a3d..fd53e0b892 100644 --- a/internal/apps/builder/cnb.go +++ b/internal/apps/builder/cnb.go @@ -2,13 +2,16 @@ package builder import ( "context" + "encoding/json" "errors" + "fmt" "os" "sort" "strings" "time" "github.com/MakeNowJust/heredoc" + "github.com/digitalocean/doctl/commands/charm" "github.com/digitalocean/doctl/commands/charm/template" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" @@ -25,9 +28,21 @@ const ( appVarPrefix = "APP_VAR_" ) -// CNBComponentBuilder represents a CNB builder +// CNBComponentBuilder represents a CNB builder. type CNBComponentBuilder struct { baseComponentBuilder + versioning CNBVersioning +} + +// CNBVersioning contains CNB versioning config. +type CNBVersioning struct { + Buildpacks []*Buildpack +} + +// Buildpack represents a CNB buildpack. +type Buildpack struct { + ID string `json:"id,omitempty"` + Version string `json:"version,omitempty"` } // Build attempts to build the requested component using the CNB Builder. @@ -41,6 +56,11 @@ func (b *CNBComponentBuilder) Build(ctx context.Context) (res ComponentBuilderRe return res, err } + env, err := b.cnbEnv() + if err != nil { + return res, fmt.Errorf("configuring environment variables: %w", err) + } + mounts := []mount.Mount{{ Type: mount.TypeBind, Source: "/var/run/docker.sock", @@ -113,7 +133,7 @@ func (b *CNBComponentBuilder) Build(ctx context.Context) (res ComponentBuilderRe execRes, err := b.cli.ContainerExecCreate(ctx, buildContainer.ID, types.ExecConfig{ AttachStderr: true, AttachStdout: true, - Env: b.cnbEnv(), + Env: env, Cmd: []string{"sh", "-c", "/.app_platform/build.sh"}, }) if err != nil { @@ -154,8 +174,11 @@ func (b *CNBComponentBuilder) Build(ctx context.Context) (res ComponentBuilderRe return res, nil } -func (b *CNBComponentBuilder) cnbEnv() []string { - envMap := b.getEnvMap() +func (b *CNBComponentBuilder) cnbEnv() ([]string, error) { + envMap, err := b.getEnvMap() + if err != nil { + return nil, err + } envs := []string{} appVars := []string{} @@ -168,6 +191,7 @@ func (b *CNBComponentBuilder) cnbEnv() []string { envs = append(envs, appVarAllowListKey+"="+strings.Join(appVars, ",")) } + envs = append(envs, "CNB_UPLOAD_RETRY=1") envs = append(envs, "APP_IMAGE_URL="+b.ImageOutputName()) envs = append(envs, "APP_PLATFORM_COMPONENT_TYPE="+string(b.component.GetType())) if b.component.GetSourceDir() != "" { @@ -175,15 +199,24 @@ func (b *CNBComponentBuilder) cnbEnv() []string { } if b.buildCommandOverride != "" { - template.Print(heredoc.Doc(` - => Overriding default build command with custom command: {{highlight .}}{{nl}}`, - ), b.buildCommandOverride) + template.Render(b.getLogWriter(), heredoc.Doc(` + {{success checkmark}} overriding default build command with custom command: + {{highlight .}} + `, + ), charm.IndentString(4, b.buildCommandOverride)) envs = append(envs, "BUILD_COMMAND="+b.buildCommandOverride) } else if b.component.GetBuildCommand() != "" { envs = append(envs, "BUILD_COMMAND="+b.component.GetBuildCommand()) } - sort.Strings(envs) + if len(b.versioning.Buildpacks) > 0 { + versioningJSON, err := json.Marshal(b.versioning.Buildpacks) + if err != nil { + return nil, fmt.Errorf("computing buildpack versioning: %w", err) + } + envs = append(envs, "VERSION_PINNING_LIST="+string(versioningJSON)) + } - return envs + sort.Strings(envs) + return envs, nil } diff --git a/internal/apps/builder/cnb_test.go b/internal/apps/builder/cnb_test.go index a5b6bcfa97..1179ec7258 100644 --- a/internal/apps/builder/cnb_test.go +++ b/internal/apps/builder/cnb_test.go @@ -111,6 +111,7 @@ func TestCNBComponentBuild(t *testing.T) { appVarPrefix + "run-build-arg-1=run-build-val-1", appVarPrefix + "useroverride-1=newval", "BUILD_COMMAND=" + builder.buildCommandOverride, + "CNB_UPLOAD_RETRY=1", "SOURCE_DIR=" + service.GetSourceDir(), }, Cmd: []string{"sh", "-c", "/.app_platform/build.sh"}, @@ -182,6 +183,7 @@ func TestCNBComponentBuild(t *testing.T) { Env: []string{ "APP_IMAGE_URL=" + builder.ImageOutputName(), "APP_PLATFORM_COMPONENT_TYPE=" + string(service.GetType()), + "CNB_UPLOAD_RETRY=1", "SOURCE_DIR=" + service.GetSourceDir(), }, Cmd: []string{"sh", "-c", "/.app_platform/build.sh"}, @@ -205,4 +207,38 @@ func TestCNBComponentBuild(t *testing.T) { _, err = builder.Build(ctx) require.NoError(t, err) }) + + t.Run("override unrecognized env", func(t *testing.T) { + ctrl := gomock.NewController(t) + service := &godo.AppServiceSpec{ + SourceDir: "./subdir", + Name: "web", + Envs: []*godo.AppVariableDefinition{ + { + Key: "build-arg-1", + Value: "build-val-1", + Type: godo.AppVariableType_General, + Scope: godo.AppVariableScope_BuildTime, + }, + }, + } + spec := &godo.AppSpec{ + Services: []*godo.AppServiceSpec{service}, + } + + mockClient := NewMockDockerEngineClient(ctrl) + builder := &CNBComponentBuilder{ + baseComponentBuilder: baseComponentBuilder{ + cli: mockClient, + spec: spec, + component: service, + envOverrides: map[string]string{ + "useroverride-1": "newval", + }, + }, + } + + _, err := builder.Build(ctx) + require.EqualError(t, err, "configuring environment variables: variable not in found in app spec: useroverride-1") + }) } diff --git a/internal/apps/builder/docker.go b/internal/apps/builder/docker.go index de84fb4d10..e385095469 100644 --- a/internal/apps/builder/docker.go +++ b/internal/apps/builder/docker.go @@ -10,7 +10,6 @@ import ( "path/filepath" "time" - "github.com/MakeNowJust/heredoc" "github.com/digitalocean/doctl/commands/charm/template" "github.com/docker/docker/api/types" "github.com/docker/docker/pkg/archive" @@ -39,14 +38,21 @@ func (b *DockerComponentBuilder) Build(ctx context.Context) (ComponentBuilderRes return res, errors.New("no component was provided for the build") } + lw := b.getLogWriter() if b.buildCommandOverride != "" { - template.Print(heredoc.Doc(` - {{warning "=> Build command overrides are ignored for Dockerfile based builds..."}}{{nl}}`, - ), b.buildCommandOverride) + template.Render(lw, + `{{warning (print crossmark " build command overrides are ignored for Dockerfile based builds")}}{{nl 2}}`, + b.buildCommandOverride, + ) + } + + buildArgs, err := b.getBuildArgs() + if err != nil { + return res, fmt.Errorf("configuring environment variables: %w", err) } buildContext := filepath.Clean(b.component.GetSourceDir()) - buildContext, err := filepath.Rel(".", buildContext) + buildContext, err = filepath.Rel(".", buildContext) if err != nil { return res, err } @@ -64,7 +70,7 @@ func (b *DockerComponentBuilder) Build(ctx context.Context) (ComponentBuilderRes Tags: []string{ b.ImageOutputName(), }, - BuildArgs: b.getBuildArgs(), + BuildArgs: buildArgs, } dockerRes, err := b.cli.ImageBuild(ctx, tar, opts) if err != nil { @@ -72,15 +78,18 @@ func (b *DockerComponentBuilder) Build(ctx context.Context) (ComponentBuilderRes return res, err } defer dockerRes.Body.Close() - print(dockerRes.Body, b.getLogWriter()) + print(dockerRes.Body, lw) res.Image = b.ImageOutputName() res.BuildDuration = time.Since(start) res.ExitCode = 0 return res, nil } -func (b *DockerComponentBuilder) getBuildArgs() map[string]*string { - envMap := b.getEnvMap() +func (b *DockerComponentBuilder) getBuildArgs() (map[string]*string, error) { + envMap, err := b.getEnvMap() + if err != nil { + return nil, err + } args := map[string]*string{} for k, v := range envMap { @@ -88,7 +97,7 @@ func (b *DockerComponentBuilder) getBuildArgs() map[string]*string { args[k] = &v } - return args + return args, nil } type dockerBuildOut struct { diff --git a/internal/apps/builder/docker_test.go b/internal/apps/builder/docker_test.go index 1982c1bc0e..34a50566a1 100644 --- a/internal/apps/builder/docker_test.go +++ b/internal/apps/builder/docker_test.go @@ -1,14 +1,17 @@ package builder import ( + "bytes" "context" "io/ioutil" "strings" "testing" + "github.com/digitalocean/doctl/commands/charm/text" "github.com/digitalocean/godo" "github.com/docker/docker/api/types" "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -72,11 +75,14 @@ func TestDockerComponentBuild(t *testing.T) { } mockClient := NewMockDockerEngineClient(ctrl) + var logBuf bytes.Buffer builder := &DockerComponentBuilder{ baseComponentBuilder: baseComponentBuilder{ - cli: mockClient, - spec: spec, - component: service, + cli: mockClient, + spec: spec, + component: service, + buildCommandOverride: "test", + logWriter: &logBuf, }, } @@ -96,5 +102,16 @@ func TestDockerComponentBuild(t *testing.T) { _, err := builder.Build(ctx) require.NoError(t, err) + + assert.Contains(t, logBuf.String(), text.Crossmark.String()+" build command overrides are ignored for Dockerfile based builds") }) } + +type logWriter struct { + t *testing.T +} + +func (lw logWriter) Write(data []byte) (int, error) { + lw.t.Log(string(data)) + return len(data), nil +}