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

driver: support for credentials helper and auth config file #265

Merged
merged 3 commits into from
Jul 6, 2023
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
18 changes: 0 additions & 18 deletions api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ package api

import (
"context"
"encoding/base64"
"encoding/json"
"fmt"
"io"
"net"
Expand All @@ -30,12 +28,6 @@ type ClientConfig struct {
HttpTimeout time.Duration
}

type ImageAuthConfig struct {
TLSVerify bool
Username string
Password string
}

func DefaultClientConfig() ClientConfig {
cfg := ClientConfig{
HttpTimeout: 60 * time.Second,
Expand Down Expand Up @@ -124,16 +116,6 @@ func (c *API) Delete(ctx context.Context, path string) (*http.Response, error) {
return c.Do(req)
}

// NewAuthHeader encodes auth configuration to a docker X-Registry-Auth header payload.
func NewAuthHeader(auth ImageAuthConfig) (string, error) {
jsonBytes, err := json.Marshal(auth)
if err != nil {
return "", err
}
header := base64.StdEncoding.EncodeToString(jsonBytes)
return header, nil
}

func ignoreClose(c io.Closer) {
_ = c.Close()
}
65 changes: 36 additions & 29 deletions api/image_pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,54 +10,61 @@ import (
"fmt"
"io"
"net/http"

"github.com/hashicorp/nomad-driver-podman/registry"
)

// ImagePull pulls a image from a remote location to local storage
func (c *API) ImagePull(ctx context.Context, nameWithTag string, auth ImageAuthConfig) (string, error) {
var id string
headers := map[string]string{}

// handle authentication
usesAuth := auth.Username != "" && auth.Password != ""
if usesAuth {
authHeader, err := NewAuthHeader(auth)
func (c *API) ImagePull(ctx context.Context, pullConfig *registry.PullConfig) (string, error) {
pullConfig.Log(c.logger)
Copy link
Contributor

Choose a reason for hiding this comment

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

This reads a bit inverted to me, but something like c.logger.Info("pull config", pullConfig.LogTags()...) looks a bit weird as well 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it's a bit unusual but it nicely hides the ~20 lines of gunk specific to the thing being logged


var (
headers = make(map[string]string)
repository = pullConfig.Image
tlsVerify = pullConfig.TLSVerify
)
Comment on lines +21 to +25
Copy link
Contributor

Choose a reason for hiding this comment

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

Just some Go curiosity, is there an advantage to declaring the variables like this instead of using the walrus :=?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope none at all; doing it this way just makes gofmt line them up nicely


// if the task or driver are configured with an auth block, attempt to find
// credentials that are compatible with the given image, and set the appropriate
// header if found
if pullConfig.AuthAvailable() {
auth, err := registry.ResolveRegistryAuthentication(repository, pullConfig)
if err != nil {
return "", err
return "", fmt.Errorf("failed to determine authentication for %q: %w", repository, err)
}
headers["X-Registry-Auth"] = authHeader
auth.SetHeader(headers)
}

c.logger.Trace("image pull details", "tls_verify", auth.TLSVerify, "reference", nameWithTag, "uses_auth", usesAuth)
urlPath := fmt.Sprintf("/v1.0.0/libpod/images/pull?reference=%s&tlsVerify=%t", repository, tlsVerify)

urlPath := fmt.Sprintf("/v1.0.0/libpod/images/pull?reference=%s&tlsVerify=%t", nameWithTag, auth.TLSVerify)
res, err := c.PostWithHeaders(ctx, urlPath, nil, headers)
if err != nil {
return "", err
return "", fmt.Errorf("failed to pull image: %w", err)
}

defer ignoreClose(res.Body)

if res.StatusCode != http.StatusOK {
body, _ := io.ReadAll(res.Body)
return "", fmt.Errorf("cannot pull image, status code: %d: %s", res.StatusCode, body)
}

dec := json.NewDecoder(res.Body)
var report ImagePullReport
for {
decErr := dec.Decode(&report)
if errors.Is(decErr, io.EOF) {
break
} else if decErr != nil {
return "", fmt.Errorf("Error reading response: %w", decErr)
}
var (
dec = json.NewDecoder(res.Body)
report ImagePullReport
id string
)

if report.Error != "" {
return "", errors.New(report.Error)
}

if report.ID != "" {
for {
decodeErr := dec.Decode(&report)
switch {
case errors.Is(decodeErr, io.EOF):
return id, nil
case decodeErr != nil:
return "", fmt.Errorf("failed to read image pull response: %w", decodeErr)
case report.Error != "":
return "", fmt.Errorf("image pull report indicates error: %s", report.Error)
case report.ID != "":
id = report.ID
}
}
return id, nil
}
9 changes: 7 additions & 2 deletions api/image_pull_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@ package api
import (
"context"
"testing"
"time"

"github.com/hashicorp/nomad-driver-podman/registry"
"github.com/shoenig/test/must"
)

func TestApi_Image_Pull(t *testing.T) {
api := newApi()
ctx := context.Background()
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
defer cancel()

testCases := []struct {
Image string
Expand All @@ -25,7 +28,9 @@ func TestApi_Image_Pull(t *testing.T) {
}

for _, testCase := range testCases {
id, err := api.ImagePull(ctx, testCase.Image, ImageAuthConfig{})
id, err := api.ImagePull(ctx, &registry.PullConfig{
Image: testCase.Image,
})
if testCase.Exists {
must.NoError(t, err)
must.NotEq(t, "", id)
Expand Down
35 changes: 25 additions & 10 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ import (
var (
// configSpec is the hcl specification returned by the ConfigSchema RPC
configSpec = hclspec.NewObject(map[string]*hclspec.Spec{
// image registry authentication options
"auth": hclspec.NewBlock("auth", false, hclspec.NewObject(map[string]*hclspec.Spec{
"config": hclspec.NewAttr("config", "string", false),
"helper": hclspec.NewAttr("helper", "string", false),
})),

// volume options
"volumes": hclspec.NewDefault(hclspec.NewBlock("volumes", false, hclspec.NewObject(map[string]*hclspec.Spec{
"enabled": hclspec.NewDefault(
Expand Down Expand Up @@ -59,6 +65,7 @@ var (
hclspec.NewLiteral("true"),
),
})),
"auth_soft_fail": hclspec.NewAttr("auth_soft_fail", "bool", false),
"command": hclspec.NewAttr("command", "string", false),
"cap_add": hclspec.NewAttr("cap_add", "list(string)", false),
"cap_drop": hclspec.NewAttr("cap_drop", "list(string)", false),
Expand Down Expand Up @@ -101,8 +108,9 @@ var (
})
)

// AuthConfig is the tasks authentication configuration
type AuthConfig struct {
// TaskAuthConfig is the tasks authentication configuration
// (there is also auth_soft_fail on the top level)
type TaskAuthConfig struct {
Username string `codec:"username"`
Password string `codec:"password"`
TLSVerify bool `codec:"tls_verify"`
Expand All @@ -125,15 +133,21 @@ type VolumeConfig struct {
SelinuxLabel string `codec:"selinuxlabel"`
}

type PluginAuthConfig struct {
FileConfig string `codec:"config"`
Helper string `codec:"helper"`
}

// PluginConfig is the driver configuration set by the SetConfig RPC call
type PluginConfig struct {
Volumes VolumeConfig `codec:"volumes"`
GC GCConfig `codec:"gc"`
RecoverStopped bool `codec:"recover_stopped"`
DisableLogCollection bool `codec:"disable_log_collection"`
SocketPath string `codec:"socket_path"`
ClientHttpTimeout string `codec:"client_http_timeout"`
ExtraLabels []string `codec:"extra_labels"`
Auth PluginAuthConfig `codec:"auth"`
Volumes VolumeConfig `codec:"volumes"`
GC GCConfig `codec:"gc"`
RecoverStopped bool `codec:"recover_stopped"`
DisableLogCollection bool `codec:"disable_log_collection"`
SocketPath string `codec:"socket_path"`
ClientHttpTimeout string `codec:"client_http_timeout"`
ExtraLabels []string `codec:"extra_labels"`
}

// LogWarnings will emit logs about known problematic configurations
Expand All @@ -147,7 +161,8 @@ func (c *PluginConfig) LogWarnings(logger hclog.Logger) {
type TaskConfig struct {
ApparmorProfile string `codec:"apparmor_profile"`
Args []string `codec:"args"`
Auth AuthConfig `codec:"auth"`
Auth TaskAuthConfig `codec:"auth"`
AuthSoftFail bool `codec:"auth_soft_fail"`
Ports []string `codec:"ports"`
Tmpfs []string `codec:"tmpfs"`
Volumes []string `codec:"volumes"`
Expand Down
33 changes: 27 additions & 6 deletions driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/containers/image/v5/types"
"github.com/hashicorp/go-hclog"
"github.com/hashicorp/nomad-driver-podman/api"
"github.com/hashicorp/nomad-driver-podman/registry"
"github.com/hashicorp/nomad-driver-podman/version"
"github.com/hashicorp/nomad/client/stats"
"github.com/hashicorp/nomad/client/taskenv"
Expand Down Expand Up @@ -693,7 +694,14 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive
return nil, nil, fmt.Errorf("failed to parse image_pull_timeout: %w", parseErr)
}

imageID, createErr := d.createImage(createOpts.Image, &driverConfig.Auth, driverConfig.ForcePull, imagePullTimeout, cfg)
imageID, createErr := d.createImage(
createOpts.Image,
&driverConfig.Auth,
driverConfig.AuthSoftFail,
driverConfig.ForcePull,
imagePullTimeout,
cfg,
)
if createErr != nil {
return nil, nil, fmt.Errorf("failed to create image: %s: %w", createOpts.Image, createErr)
}
Expand Down Expand Up @@ -906,7 +914,14 @@ func sliceMergeUlimit(ulimitsRaw map[string]string) ([]spec.POSIXRlimit, error)

// Creates the requested image if missing from storage
// returns the 64-byte image ID as an unique image identifier
func (d *Driver) createImage(image string, auth *AuthConfig, forcePull bool, imagePullTimeout time.Duration, cfg *drivers.TaskConfig) (string, error) {
func (d *Driver) createImage(
image string,
auth *TaskAuthConfig,
authSoftFail bool,
forcePull bool,
imagePullTimeout time.Duration,
cfg *drivers.TaskConfig,
) (string, error) {
var imageID string
imageName := image
// If it is a shortname, we should not have to worry
Expand Down Expand Up @@ -961,18 +976,24 @@ func (d *Driver) createImage(image string, auth *AuthConfig, forcePull bool, ima
Message: "Pulling image " + imageName,
})

imageAuth := api.ImageAuthConfig{
pc := &registry.PullConfig{
Image: imageName,
TLSVerify: auth.TLSVerify,
Username: auth.Username,
Password: auth.Password,
RegistryConfig: &registry.RegistryAuthConfig{
Username: auth.Username,
Password: auth.Password,
},
CredentialsFile: d.config.Auth.FileConfig,
CredentialsHelper: d.config.Auth.Helper,
AuthSoftFail: authSoftFail,
}

result, err, _ := d.pullGroup.Do(imageName, func() (interface{}, error) {

ctx, cancel := context.WithTimeout(context.Background(), imagePullTimeout)
defer cancel()

if imageID, err = d.slowPodman.ImagePull(ctx, imageName, imageAuth); err != nil {
if imageID, err = d.slowPodman.ImagePull(ctx, pc); err != nil {
return imageID, fmt.Errorf("failed to start task, unable to pull image %s : %w", imageName, err)
}
return imageID, nil
Expand Down
6 changes: 3 additions & 3 deletions driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1983,7 +1983,7 @@ func startDestroyInspectImage(t *testing.T, image string, taskName string) {
AllocID: uuid.Generate(),
Resources: createBasicResources(),
}
imageID, err := getPodmanDriver(t, d).createImage(image, &AuthConfig{}, false, 5*time.Minute, task)
imageID, err := getPodmanDriver(t, d).createImage(image, &TaskAuthConfig{}, false, false, 5*time.Minute, task)
must.NoError(t, err)
must.Eq(t, imageID, inspectData.Image)
}
Expand Down Expand Up @@ -2063,7 +2063,7 @@ insecure = true`
go func() {
// Pull image using our proxy.
image := "localhost:5000/quay/busybox:latest"
_, err = getPodmanDriver(t, d).createImage(image, &AuthConfig{}, true, 3*time.Second, task)
_, err = getPodmanDriver(t, d).createImage(image, &TaskAuthConfig{}, false, true, 3*time.Second, task)
resultCh <- err
}()

Expand Down Expand Up @@ -2141,7 +2141,7 @@ func createInspectImage(t *testing.T, image, reference string) {
AllocID: uuid.Generate(),
Resources: createBasicResources(),
}
idTest, err := getPodmanDriver(t, d).createImage(image, &AuthConfig{}, false, 5*time.Minute, task)
idTest, err := getPodmanDriver(t, d).createImage(image, &TaskAuthConfig{}, false, false, 5*time.Minute, task)
must.NoError(t, err)

idRef, err := getPodmanDriver(t, d).podman.ImageInspectID(context.Background(), reference)
Expand Down
Loading