feat: added support for private registries on docker (#2058)

This adds support for private container registries TODO - [x] add docs - [x] cleanup all the nils and maybe make the FetchImage interface nicer - [x] perhaps add e2e tests (skipped) Closes #1342 --------- Co-authored-by: Tedi Mitiku <[email protected]>
kurtosis-tech · Jan 22, 2024 · 7cda3d0 · 7cda3d0
1 parent f0f51b4
commit 7cda3d0
Show file tree

Hide file tree

Showing 30 changed files with 552 additions and 87 deletions.
diff --git a/...er-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/docker_kurtosis_backend.go b/...er-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/docker_kurtosis_backend.go
@@ -3,6 +3,7 @@ package docker_kurtosis_backend
 import (
 	"context"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_build_spec"
+	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_registry_spec"
 	"io"
 	"sync"
 
@@ -84,8 +85,8 @@ func NewDockerKurtosisBackend(
 	}
 }
 
-func (backend *DockerKurtosisBackend) FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error) {
-	return backend.dockerManager.FetchImage(ctx, image, downloadMode)
+func (backend *DockerKurtosisBackend) FetchImage(ctx context.Context, image string, registrySpec *image_registry_spec.ImageRegistrySpec, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error) {
+	return backend.dockerManager.FetchImage(ctx, image, registrySpec, downloadMode)
 }
 
 func (backend *DockerKurtosisBackend) PruneUnusedImages(ctx context.Context) ([]string, error) {

diff --git a/...d_impls/docker/docker_kurtosis_backend/docker_kurtosis_backend_api_container_functions.go b/...d_impls/docker/docker_kurtosis_backend/docker_kurtosis_backend_api_container_functions.go
@@ -2,6 +2,7 @@ package docker_kurtosis_backend
 
 import (
 	"context"
+	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_registry_spec"
 	"net"
 	"time"
 
@@ -34,6 +35,8 @@ const (
 
 // TODO: MIGRATE THIS FOLDER TO USE STRUCTURE OF USER_SERVICE_FUNCTIONS MODULE
 
+var emptyRegistrySpecAsPublicImage *image_registry_spec.ImageRegistrySpec = nil
+
 func (backend *DockerKurtosisBackend) CreateAPIContainer(
 	ctx context.Context,
 	image string,
@@ -183,7 +186,7 @@ func (backend *DockerKurtosisBackend) CreateAPIContainer(
 		labelStrs,
 	).WithRestartPolicy(docker_manager.RestartOnFailure).Build()
 
-	if _, err = backend.dockerManager.FetchImageIfMissing(ctx, image); err != nil {
+	if _, err = backend.dockerManager.FetchImageIfMissing(ctx, image, emptyRegistrySpecAsPublicImage); err != nil {
 		logrus.Warnf("Failed to pull the latest version of API container image '%v'; you may be running an out-of-date version. Error:\n%v", image, err)
 	}
 

diff --git a/...ner-engine-lib/lib/backend_impls/docker/docker_manager/create_and_start_container_args.go b/...ner-engine-lib/lib/backend_impls/docker/docker_manager/create_and_start_container_args.go
@@ -1,6 +1,7 @@
 package docker_manager
 
 import (
+	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_registry_spec"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/service_user"
 	"net"
 
@@ -34,6 +35,7 @@ type CreateAndStartContainerArgs struct {
 	restartPolicy                            RestartPolicy
 	imageDownloadMode                        image_download_mode.ImageDownloadMode
 	user                                     *service_user.ServiceUser
+	imageRegistrySpec                        *image_registry_spec.ImageRegistrySpec
 }
 
 // Builder for creating CreateAndStartContainerArgs object
@@ -62,6 +64,7 @@ type CreateAndStartContainerArgsBuilder struct {
 	restartPolicy                            RestartPolicy
 	imageDownloadMode                        image_download_mode.ImageDownloadMode
 	user                                     *service_user.ServiceUser
+	imageRegistrySpec                        *image_registry_spec.ImageRegistrySpec
 }
 
 /*
@@ -97,6 +100,7 @@ func NewCreateAndStartContainerArgsBuilder(dockerImage string, name string, netw
 		restartPolicy:                            NoRestart,
 		imageDownloadMode:                        image_download_mode.ImageDownloadMode_Missing,
 		user:                                     nil,
+		imageRegistrySpec:                        nil,
 	}
 }
 
@@ -126,6 +130,7 @@ func (builder *CreateAndStartContainerArgsBuilder) Build() *CreateAndStartContai
 		restartPolicy:                            builder.restartPolicy,
 		imageDownloadMode:                        builder.imageDownloadMode,
 		user:                                     builder.user,
+		imageRegistrySpec:                        builder.imageRegistrySpec,
 	}
 }
 
@@ -275,3 +280,8 @@ func (builder *CreateAndStartContainerArgsBuilder) WithUser(user *service_user.S
 	builder.user = user
 	return builder
 }
+
+func (builder *CreateAndStartContainerArgsBuilder) WithImageRegistrySpec(imageRegistrySpec *image_registry_spec.ImageRegistrySpec) *CreateAndStartContainerArgsBuilder {
+	builder.imageRegistrySpec = imageRegistrySpec
+	return builder
+}
diff --git a/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_manager.go b/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_manager.go
@@ -14,6 +14,7 @@ import (
 	"github.com/docker/docker/api/types/registry"
 	"github.com/docker/go-units"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_build_spec"
+	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_registry_spec"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/uuid_generator"
 	"github.com/kurtosis-tech/kurtosis/utils"
 	"io"
@@ -527,7 +528,7 @@ func (manager *DockerManager) CreateAndStartContainer(
 		dockerImage = dockerImage + dockerTagSeparatorChar + dockerDefaultTag
 	}
 
-	_, _, err := manager.FetchImage(ctx, dockerImage, args.imageDownloadMode)
+	_, _, err := manager.FetchImage(ctx, dockerImage, args.imageRegistrySpec, args.imageDownloadMode)
 	if err != nil {
 		logrus.Debugf("Error occurred fetching image '%v'. Err:\n%v", dockerImage, err)
 		return "", nil, stacktrace.Propagate(err, "An error occurred fetching image '%v'", dockerImage)
@@ -1223,7 +1224,7 @@ func (manager *DockerManager) GetContainersByNetworkId(ctx context.Context, netw
 // [FetchImageIfMissing] uses the local [dockerImage] if it's available.
 // If unavailable, will attempt to fetch the latest image.
 // Returns error if local [dockerImage] is unavailable and pulling image fails.
-func (manager *DockerManager) FetchImageIfMissing(ctx context.Context, dockerImage string) (bool, error) {
+func (manager *DockerManager) FetchImageIfMissing(ctx context.Context, dockerImage string, registrySpec *image_registry_spec.ImageRegistrySpec) (bool, error) {
 	// if the image name doesn't have version information we concatenate `:latest`
 	// this behavior is similar to CreateAndStartContainer above
 	// this allows us to be deterministic in our behaviour
@@ -1239,7 +1240,7 @@ func (manager *DockerManager) FetchImageIfMissing(ctx context.Context, dockerIma
 
 	if !doesImageExistLocally {
 		logrus.Tracef("Image doesn't exist locally, so attempting to pull it...")
-		err = manager.pullImage(ctx, dockerImage)
+		err = manager.pullImage(ctx, dockerImage, registrySpec)
 		if err != nil {
 			return false, stacktrace.Propagate(err, "Failed to pull Docker image '%v' from remote image repository", dockerImage)
 		}
@@ -1252,7 +1253,7 @@ func (manager *DockerManager) FetchImageIfMissing(ctx context.Context, dockerIma
 // [FetchLatestImage] always attempts to retrieve the latest [dockerImage].
 // If retrieving the latest [dockerImage] fails, the local image will be used.
 // Returns error, if no local image is available after retrieving latest fails.
-func (manager *DockerManager) FetchLatestImage(ctx context.Context, dockerImage string) error {
+func (manager *DockerManager) FetchLatestImage(ctx context.Context, dockerImage string, registrySpec *image_registry_spec.ImageRegistrySpec) error {
 	// if the image name doesn't have version information we concatenate `:latest`
 	// this behavior is similar to CreateAndStartContainer above
 	// this allows us to be deterministic in our behaviour
@@ -1269,14 +1270,14 @@ func (manager *DockerManager) FetchLatestImage(ctx context.Context, dockerImage
 	// try and pull latest image even if image exists locally
 	if doesImageExistLocally {
 		logrus.Tracef("Image exists locally, but attempting to get latest from remote image repository.")
-		err = manager.pullImage(ctx, dockerImage)
+		err = manager.pullImage(ctx, dockerImage, registrySpec)
 		if err != nil {
 			logrus.Tracef("Failed to pull Docker image '%v' from remote image repository. Going to use available local image.", dockerImage)
 		} else {
 			logrus.Tracef("Latest image successfully pulled from remote to local.")
 		}
 	} else {
-		err = manager.pullImage(ctx, dockerImage)
+		err = manager.pullImage(ctx, dockerImage, registrySpec)
 		if err != nil {
 			return stacktrace.Propagate(err, "Failed to pull Docker image '%v' from remote image repository.", dockerImage)
 		}
@@ -1285,16 +1286,16 @@ func (manager *DockerManager) FetchLatestImage(ctx context.Context, dockerImage
 	return nil
 }
 
-func (manager *DockerManager) FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error) {
+func (manager *DockerManager) FetchImage(ctx context.Context, image string, registrySpec *image_registry_spec.ImageRegistrySpec, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error) {
 	var err error
 	var pulledFromRemote bool = true
 	logrus.Debugf("Fetching image '%s' with image download mode: %s", image, downloadMode)
 
 	switch image_fetching := downloadMode; image_fetching {
 	case image_download_mode.ImageDownloadMode_Always:
-		err = manager.FetchLatestImage(ctx, image)
+		err = manager.FetchLatestImage(ctx, image, registrySpec)
 	case image_download_mode.ImageDownloadMode_Missing:
-		pulledFromRemote, err = manager.FetchImageIfMissing(ctx, image)
+		pulledFromRemote, err = manager.FetchImageIfMissing(ctx, image, registrySpec)
 	default:
 		return false, "", stacktrace.NewError("Undefined image pulling mode: '%v'", image_fetching)
 	}
@@ -1546,14 +1547,14 @@ func (manager *DockerManager) isImageAvailableLocally(imageName string) (bool, e
 	return numMatchingImages > 0, nil
 }
 
-func (manager *DockerManager) pullImage(context context.Context, imageName string) error {
+func (manager *DockerManager) pullImage(context context.Context, imageName string, registrySpec *image_registry_spec.ImageRegistrySpec) error {
 	// As we're using the docker client with no timeout to pull the image, we quickly check with the client that has
 	// a timeout whether the docker engine is reachable.
 	if _, err := manager.dockerClient.Ping(context); err != nil {
 		return stacktrace.Propagate(err, "An error occurred communicating with docker engine")
 	}
 	logrus.Infof("Pulling image '%s'", imageName)
-	err, retryWithLinuxAmd64 := pullImage(manager.dockerClientNoTimeout, imageName, defaultPlatform)
+	err, retryWithLinuxAmd64 := pullImage(manager.dockerClientNoTimeout, imageName, registrySpec, defaultPlatform)
 	if err == nil {
 		return nil
 	}
@@ -1562,7 +1563,7 @@ func (manager *DockerManager) pullImage(context context.Context, imageName strin
 	}
 	// we retry with linux/amd64
 	logrus.Debugf("Retrying pulling image '%s' for '%s'", imageName, linuxAmd64)
-	err, _ = pullImage(manager.dockerClientNoTimeout, imageName, linuxAmd64)
+	err, _ = pullImage(manager.dockerClientNoTimeout, imageName, registrySpec, linuxAmd64)
 	if err != nil {
 		return stacktrace.Propagate(err, "Had previously failed with a manifest error so tried pulling image '%v' for platform '%v' but failed", imageName, linuxAmd64)
 	}
@@ -2170,17 +2171,35 @@ func getEndpointSettingsForIpAddress(ipAddress string, alias string) *network.En
 	return config
 }
 
-func pullImage(dockerClient *client.Client, imageName string, platform string) (error, bool) {
+func pullImage(dockerClient *client.Client, imageName string, registrySpec *image_registry_spec.ImageRegistrySpec, platform string) (error, bool) {
 	// Own context for pulling images because we do not want to cancel this works in case the main context in the request is cancelled
 	// if the fist request fails the image will be ready for following request making the process faster
 	pullImageCtx := context.Background()
 	logrus.Tracef("Starting pulling '%s' for platform '%s'", imageName, platform)
-	out, err := dockerClient.ImagePull(pullImageCtx, imageName, types.ImagePullOptions{
+	imagePullOptions := types.ImagePullOptions{
 		All:           false,
 		RegistryAuth:  "",
 		PrivilegeFunc: nil,
 		Platform:      platform,
-	})
+	}
+	if registrySpec != nil {
+		authConfig := registry.AuthConfig{
+			Username:      registrySpec.GetUsername(),
+			Password:      registrySpec.GetPassword(),
+			Email:         "",
+			Auth:          "",
+			ServerAddress: registrySpec.GetRegistryAddr(),
+			IdentityToken: "",
+			RegistryToken: "",
+		}
+		encodedAuthConfig, err := registry.EncodeAuthConfig(authConfig)
+		if err != nil {
+			return stacktrace.Propagate(err, "An error occurred while converting registry auth to base64"), false
+		}
+		imagePullOptions.RegistryAuth = encodedAuthConfig
+
+	}
+	out, err := dockerClient.ImagePull(pullImageCtx, imageName, imagePullOptions)
 	if err != nil {
 		return stacktrace.Propagate(err, "Tried pulling image '%v' with platform '%v' but failed", imageName, platform), false
 	}

diff --git a/...b/lib/backend_impls/kubernetes/kubernetes_kurtosis_backend/kubernetes_kurtosis_backend.go b/...b/lib/backend_impls/kubernetes/kubernetes_kurtosis_backend/kubernetes_kurtosis_backend.go
@@ -3,6 +3,7 @@ package kubernetes_kurtosis_backend
 import (
 	"context"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_build_spec"
+	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_registry_spec"
 	"io"
 	apiv1 "k8s.io/api/core/v1"
 
@@ -114,7 +115,7 @@ func NewCLIModeKubernetesKurtosisBackend(
 	)
 }
 
-func (backend *KubernetesKurtosisBackend) FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error) {
+func (backend *KubernetesKurtosisBackend) FetchImage(ctx context.Context, image string, registrySpec *image_registry_spec.ImageRegistrySpec, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error) {
 	logrus.Warnf("FetchImage isn't implemented for Kubernetes yet")
 	return false, "", nil
 }

diff --git a/container-engine-lib/lib/backend_impls/kubernetes/kubernetes_manager/kubernetes_manager.go b/container-engine-lib/lib/backend_impls/kubernetes/kubernetes_manager/kubernetes_manager.go
@@ -1089,27 +1089,29 @@ func (manager *KubernetesManager) CreatePod(
 		HostIPC:                       false,
 		ShareProcessNamespace:         nil,
 		SecurityContext:               nil,
-		ImagePullSecrets:              nil,
-		Hostname:                      "",
-		Subdomain:                     "",
-		Affinity:                      nil,
-		SchedulerName:                 "",
-		Tolerations:                   nil,
-		HostAliases:                   nil,
-		PriorityClassName:             "",
-		Priority:                      nil,
-		DNSConfig:                     nil,
-		ReadinessGates:                nil,
-		RuntimeClassName:              nil,
-		EnableServiceLinks:            nil,
-		PreemptionPolicy:              nil,
-		Overhead:                      nil,
-		TopologySpreadConstraints:     nil,
-		SetHostnameAsFQDN:             nil,
-		OS:                            nil,
-		HostUsers:                     nil,
-		SchedulingGates:               nil,
-		ResourceClaims:                nil,
+		// TODO add support for ImageRegistrySpec to Kubernetes by adding the right secret here
+		// You will have to first publish the secret using the Kubernetes API
+		ImagePullSecrets:          nil,
+		Hostname:                  "",
+		Subdomain:                 "",
+		Affinity:                  nil,
+		SchedulerName:             "",
+		Tolerations:               nil,
+		HostAliases:               nil,
+		PriorityClassName:         "",
+		Priority:                  nil,
+		DNSConfig:                 nil,
+		ReadinessGates:            nil,
+		RuntimeClassName:          nil,
+		EnableServiceLinks:        nil,
+		PreemptionPolicy:          nil,
+		Overhead:                  nil,
+		TopologySpreadConstraints: nil,
+		SetHostnameAsFQDN:         nil,
+		OS:                        nil,
+		HostUsers:                 nil,
+		SchedulingGates:           nil,
+		ResourceClaims:            nil,
 	}
 
 	podToCreate := &apiv1.Pod{

diff --git a/...iner-engine-lib/lib/backend_impls/metrics_reporting/metrics_reporting_kurtosis_backend.go b/...iner-engine-lib/lib/backend_impls/metrics_reporting/metrics_reporting_kurtosis_backend.go
@@ -3,6 +3,7 @@ package metrics_reporting
 import (
 	"context"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_build_spec"
+	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_registry_spec"
 	"io"
 	"time"
 
@@ -29,8 +30,8 @@ func NewMetricsReportingKurtosisBackend(underlying backend_interface.KurtosisBac
 	return &MetricsReportingKurtosisBackend{underlying: underlying}
 }
 
-func (backend *MetricsReportingKurtosisBackend) FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error) {
-	pulledFromRemote, architecture, err := backend.underlying.FetchImage(ctx, image, downloadMode)
+func (backend *MetricsReportingKurtosisBackend) FetchImage(ctx context.Context, image string, registrySpec *image_registry_spec.ImageRegistrySpec, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error) {
+	pulledFromRemote, architecture, err := backend.underlying.FetchImage(ctx, image, registrySpec, downloadMode)
 	if err != nil {
 		return false, "", stacktrace.Propagate(err, "An error occurred pulling image '%v'", image)
 	}

diff --git a/container-engine-lib/lib/backend_interface/kurtosis_backend.go b/container-engine-lib/lib/backend_interface/kurtosis_backend.go
@@ -3,6 +3,7 @@ package backend_interface
 import (
 	"context"
 	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_build_spec"
+	"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_registry_spec"
 	"io"
 	"time"
 
@@ -34,7 +35,7 @@ type KurtosisBackend interface {
 	// If retrieving the latest [dockerImage] fails, the local image will be used.
 	// Returns True is it was retrieved from cloud or False if it's a local image
 	// Returns a string that represents the architecture of the image
-	FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error)
+	FetchImage(ctx context.Context, image string, registrySpec *image_registry_spec.ImageRegistrySpec, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error)
 
 	PruneUnusedImages(ctx context.Context) ([]string, error)