Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addition of experimental flag full-control-dockerfile #3114

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions docs/content/docs/configuration/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ You may set a default value for a configuration value in the config file, overri
- [Build Drivers](#build-drivers)
- [Structured Logs](#structured-logs)
- [Dependencies v2](#dependencies-v2)
- [Full control Dockerfile](#full-control-dockerfile)
- [Common Configuration Settings](#common-configuration-settings)
- [Set Current Namespace](#namespace)
- [Output Formatting](#output)
Expand Down Expand Up @@ -278,6 +279,15 @@ telemetry:
The `dependencies-v2` experimental flag is not yet implemented.
When it is completed, it is used to activate the features from [PEP003 - Advanced Dependencies](https://github.com/getporter/proposals/blob/main/pep/003-advanced-dependencies.md).

### Full control Dockerfile

The `full-control-dockerfile` experimental flag disables all Dockerfile generation when building bundles.
When enabled Porter will use the file referenced by `dockerfile` in the Porter manifest when building the invocation image *without modifying* it in any way.
Ie. Porter will not process `# PORTER_x` placeholders, nor inject any user configuration and `CMD` statements.
It is up to the bundle author to ensure that the contents of the Dockerfile contains the necessary tools for any mixins to function and a layout that can be executed as a Porter bundle.

*Note:* when using the `dockerfile` property in the Porter manifest the `autobuild` functionality does not re-build the bundle on changes to the contents of the referenced file.

## Common Configuration Settings

Some configuration settings are applicable to many of Porter's commands and to save time you may want to set these values in the configuration file or with environment variables.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ You may set a default value for a configuration value in the config file, overri
- [Build Drivers](#build-drivers)
- [Structured Logs](#structured-logs)
- [Dependencies v2](#dependencies-v2)
- [Full control Dockerfile](#full-control-dockerfile)
- [Common Configuration Settings](#common-configuration-settings)
- [Set Current Namespace](#namespace)
- [Output Formatting](#output)
Expand Down Expand Up @@ -275,6 +276,14 @@ telemetry:
The `dependencies-v2` experimental flag is not yet implemented.
When it is completed, it is used to activate the features from [PEP003 - Advanced Dependencies](https://github.com/getporter/proposals/blob/main/pep/003-dependency-namespaces-and-labels.md).

### Full control Dockerfile

The `full-control-dockerfile` experimental flag disables all Dockerfile generation when building bundles.
Copy link
Member

Choose a reason for hiding this comment

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

We should maybe document sane things Porter needs to run (like copying the bundle directory, or permissions that we have in the templated Dockerfile)

Copy link
Contributor Author

@jarnfast jarnfast May 15, 2024

Choose a reason for hiding this comment

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

Hm.. maybe? I'm not super convinced - it is a "here be dragons" experimental flag. 🐲
How about linking to https://porter.sh/docs/introduction/concepts-and-components/intro-invocation-images/ which actually has some nice descriptions of what's going on. Would that be enough? 😺

Copy link
Member

Choose a reason for hiding this comment

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

I think just pointing out you need the following for Porter to be able to run:

# PORTER_INIT

RUN rm -f /etc/apt/apt.conf.d/docker-clean; echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' > /etc/apt/apt.conf.d/keep-cache
RUN --mount=type=cache,target=/var/cache/apt --mount=type=cache,target=/var/lib/apt \
    apt-get update && apt-get install -y ca-certificates

# PORTER_MIXINS

# Use the BUNDLE_DIR build argument to copy files into the bundle's working directory
COPY --link . ${BUNDLE_DIR}

In your test example you are still copying bundleDir into /cnab/app in the container, and I think it's just a good call out to say "Porter will not run, with this flag, if you don't copy the bundle directory"
We can still link to the intro to invocation images, too. But it makes using the flag a little more "usable" to those who aren't Porter nerds!

When enabled Porter will use the file referenced by `dockerfile` in the Porter manifest when building the invocation image *without modifying* it in any way.
Ie. Porter will not process `# PORTER_x` placeholders, nor inject any user configuration and `CMD` statements.
It is up to the bundle author to ensure that the contents of the Dockerfile contains the necessary tools for any mixins to function and a layout that can be executed as a Porter bundle.

*Note:* when using the `dockerfile` property in the Porter manifest the `autobuild` functionality does not re-build the bundle on changes to the contents of the referenced file.
## Common Configuration Settings

Some configuration settings are applicable to many of Porter's commands and to save time you may want to set these values in the configuration file or with environment variables.
Expand Down
46 changes: 43 additions & 3 deletions pkg/build/dockerfile-generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import (
"bufio"
"bytes"
"context"
"errors"
"fmt"
"io"
"path/filepath"
"strings"

"get.porter.sh/porter/pkg"
"get.porter.sh/porter/pkg/config"
"get.porter.sh/porter/pkg/experimental"
"get.porter.sh/porter/pkg/manifest"
"get.porter.sh/porter/pkg/mixin/query"
"get.porter.sh/porter/pkg/pkgmgmt"
Expand Down Expand Up @@ -45,9 +47,20 @@ func (g *DockerfileGenerator) GenerateDockerFile(ctx context.Context) error {
ctx, span := tracing.StartSpan(ctx)
defer span.EndSpan()

lines, err := g.buildDockerfile(ctx)
if err != nil {
return span.Error(fmt.Errorf("error generating the Dockerfile: %w", err))
var lines []string
var err error
if g.Config.IsFeatureEnabled(experimental.FlagFullControlDockerfile) {
span.Warnf("WARNING: Experimental feature \"%s\" enabled: Dockerfile will be used without changes by Porter",
experimental.FullControlDockerfile)
lines, err = g.readRawDockerfile(ctx)
if err != nil {
return span.Error(fmt.Errorf("error reading the Dockerfile: %w", err))
}
} else {
schristoff marked this conversation as resolved.
Show resolved Hide resolved
lines, err = g.buildDockerfile(ctx)
if err != nil {
return span.Error(fmt.Errorf("error generating the Dockerfile: %w", err))
}
}

contents := strings.Join(lines, "\n")
Expand All @@ -63,6 +76,33 @@ func (g *DockerfileGenerator) GenerateDockerFile(ctx context.Context) error {
return nil
}

func (g *DockerfileGenerator) readRawDockerfile(ctx context.Context) ([]string, error) {
if g.Manifest.Dockerfile == "" {
return nil, errors.New("no Dockerfile specified in the manifest")
}
exists, err := g.FileSystem.Exists(g.Manifest.Dockerfile)
if err != nil {
return nil, fmt.Errorf("error checking if Dockerfile exists: %q: %w", g.Manifest.Dockerfile, err)
}
if !exists {
return nil, fmt.Errorf("the Dockerfile specified in the manifest doesn't exist: %q", g.Manifest.Dockerfile)
}

file, err := g.FileSystem.Open(g.Manifest.Dockerfile)
if err != nil {
return nil, err
}
defer file.Close()

var lines []string
scanner := bufio.NewScanner(file)
for scanner.Scan() {
lines = append(lines, scanner.Text())
}

return lines, scanner.Err()
}

func (g *DockerfileGenerator) buildDockerfile(ctx context.Context) ([]string, error) {
log := tracing.LoggerFromContext(ctx)
log.Debug("Generating Dockerfile")
Expand Down
111 changes: 111 additions & 0 deletions pkg/build/dockerfile-generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"testing"

"get.porter.sh/porter/pkg/config"
"get.porter.sh/porter/pkg/experimental"
"get.porter.sh/porter/pkg/manifest"
"get.porter.sh/porter/pkg/mixin"
"get.porter.sh/porter/pkg/templates"
Expand Down Expand Up @@ -259,3 +260,113 @@ func TestPorter_buildMixinsSection_mixinErr(t *testing.T) {
_, err = g.buildMixinsSection(ctx)
require.EqualError(t, err, "1 error occurred:\n\t* error encountered from mixin \"exec\": encountered build error\n\n")
}

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

ctx := context.Background()
c := config.NewTestConfig(t)

// Enable the experimental feature
c.SetExperimentalFlags(experimental.FlagFullControlDockerfile)

defer c.Close()

// Start a span so we can capture the output
ctx, log := c.StartRootSpan(ctx, t.Name())
defer log.Close()

tmpl := templates.NewTemplates(c.Config)
configTpl, err := tmpl.GetManifest()
require.Nil(t, err)
c.TestContext.AddTestFileContents(configTpl, config.Name)

m, err := manifest.LoadManifestFrom(ctx, c.Config, config.Name)
require.NoError(t, err, "could not load manifest")

// Use a custom dockerfile template
m.Dockerfile = "Dockerfile.template"
customFrom := `FROM ubuntu:latest
# stuff
RUN random-command
# PORTER_INIT
COPY mybin /cnab/app/
# No attempts are made to validate the contents
# Nor does it modify the contents in any way`
c.TestContext.AddTestFileContents([]byte(customFrom), "Dockerfile.template")

mp := mixin.NewTestMixinProvider()
g := NewDockerfileGenerator(c.Config, m, tmpl, mp)
err = g.GenerateDockerFile(ctx)
require.NoError(t, err)

wantDockerfilePath := ".cnab/Dockerfile"
gotDockerfile, err := c.FileSystem.ReadFile(wantDockerfilePath)
require.NoError(t, err)

// Verify that we logged the dockerfile contents
tests.RequireOutputContains(t, c.TestContext.GetError(), string(gotDockerfile), "expected the dockerfile to be printed to the logs")
assert.Equal(t, customFrom, string(gotDockerfile))
}

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

ctx := context.Background()
c := config.NewTestConfig(t)

// Enable the experimental feature
c.SetExperimentalFlags(experimental.FlagFullControlDockerfile)

defer c.Close()

// Start a span so we can capture the output
ctx, log := c.StartRootSpan(ctx, t.Name())
defer log.Close()

tmpl := templates.NewTemplates(c.Config)
configTpl, err := tmpl.GetManifest()
require.Nil(t, err)
c.TestContext.AddTestFileContents(configTpl, config.Name)

m, err := manifest.LoadManifestFrom(ctx, c.Config, config.Name)
require.NoError(t, err, "could not load manifest")

mp := mixin.NewTestMixinProvider()
g := NewDockerfileGenerator(c.Config, m, tmpl, mp)
err = g.GenerateDockerFile(ctx)

require.EqualError(t, err, "error reading the Dockerfile: no Dockerfile specified in the manifest")
}

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

ctx := context.Background()
c := config.NewTestConfig(t)

// Enable the experimental feature
c.SetExperimentalFlags(experimental.FlagFullControlDockerfile)

defer c.Close()

// Start a span so we can capture the output
ctx, log := c.StartRootSpan(ctx, t.Name())
defer log.Close()

tmpl := templates.NewTemplates(c.Config)
configTpl, err := tmpl.GetManifest()
require.Nil(t, err)
c.TestContext.AddTestFileContents(configTpl, config.Name)

m, err := manifest.LoadManifestFrom(ctx, c.Config, config.Name)
require.NoError(t, err, "could not load manifest")

m.Dockerfile = "this-file-does-not-exist.dockerfile"

mp := mixin.NewTestMixinProvider()
g := NewDockerfileGenerator(c.Config, m, tmpl, mp)
err = g.GenerateDockerFile(ctx)

require.EqualError(t, err, "error reading the Dockerfile: the Dockerfile specified in the manifest doesn't exist: \"this-file-does-not-exist.dockerfile\"")
}
8 changes: 8 additions & 0 deletions pkg/experimental/experimental.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ const (

// DependenciesV2 is the name of the experimental feature flag for PEP003 - Advanced Dependencies.
DependenciesV2 = "dependencies-v2"

// FullControlDockerfile is the name of the experimental feature flag giving authors full control of the invocation image Dockerfile
FullControlDockerfile = "full-control-dockerfile"
)

// FeatureFlags is an enum of possible feature flags
Expand All @@ -17,6 +20,9 @@ const (

// FlagDependenciesV2 gates the changes from PEP003 - Advanced Dependencies.
FlagDependenciesV2

// FlagFullControlDockerfile gates the changes required for giving authors full control of the invocation image Dockerfile
FlagFullControlDockerfile
)

// ParseFlags converts a list of feature flag names into a bit map for faster lookups.
Expand All @@ -28,6 +34,8 @@ func ParseFlags(flags []string) FeatureFlags {
experimental = experimental | FlagNoopFeature
case DependenciesV2:
experimental = experimental | FlagDependenciesV2
case FullControlDockerfile:
experimental = experimental | FlagFullControlDockerfile
}
}
return experimental
Expand Down
Loading