From 865a899f70d485cffec04f6a9ea08bc822c2e378 Mon Sep 17 00:00:00 2001
From: Christian Kellner <christian@kellner.me>
Date: Sun, 10 Jul 2022 21:32:05 +0200
Subject: [PATCH] container/client: rework tls settings

Instead of having an extra field, `TlsVerify`, on the `Client` and
then later setting the corresponding `SystemContext` options, use
the existing `SystemContext` field of `Client`. The corresponding
field is a tri-state: unset, true, false, which is represented as
a pointer to boolean in the `Client`'s new getter and setter. This
also inverts the boolean logic from verify TLS to skip TLS which
aligns very well with the corresponding fields in the upload target
struct.
In addition we properly capitalize some existing variables.
---
 cmd/osbuild-upload-container/main.go  |  8 ++++---
 cmd/osbuild-worker/jobimpl-osbuild.go |  5 +---
 internal/container/client.go          | 33 ++++++++++++++++++++++++---
 3 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/cmd/osbuild-upload-container/main.go b/cmd/osbuild-upload-container/main.go
index c2f927b91..61a329cd5 100644
--- a/cmd/osbuild-upload-container/main.go
+++ b/cmd/osbuild-upload-container/main.go
@@ -17,14 +17,14 @@ func main() {
 	var username string
 	var password string
 	var tag string
-	var ignoreTls bool
+	var ignoreTLS bool
 
 	flag.StringVar(&filename, "container", "", "path to the oci-archive to upload (required)")
 	flag.StringVar(&destination, "destination", "", "destination to upload to (required)")
 	flag.StringVar(&tag, "tag", "", "destination tag to use for the container")
 	flag.StringVar(&username, "username", "", "username to use for registry")
 	flag.StringVar(&password, "password", "", "password to use for registry")
-	flag.BoolVar(&ignoreTls, "ignore-tls", false, "ignore tls verification for destination")
+	flag.BoolVar(&ignoreTLS, "ignore-tls", false, "ignore tls verification for destination")
 	flag.Parse()
 
 	if filename == "" || destination == "" {
@@ -59,7 +59,9 @@ func main() {
 		client.SetCredentials(username, password)
 	}
 
-	client.TlsVerify = !ignoreTls
+	if ignoreTLS {
+		client.SkipTLSVerify()
+	}
 
 	ctx := context.Background()
 
diff --git a/cmd/osbuild-worker/jobimpl-osbuild.go b/cmd/osbuild-worker/jobimpl-osbuild.go
index f055f2068..c53d7e281 100644
--- a/cmd/osbuild-worker/jobimpl-osbuild.go
+++ b/cmd/osbuild-worker/jobimpl-osbuild.go
@@ -751,10 +751,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error {
 
 			client.Auth.Username = targetOptions.Username
 			client.Auth.Password = targetOptions.Password
-
-			if targetOptions.TlsVerify != nil {
-				client.TlsVerify = *targetOptions.TlsVerify
-			}
+			client.SetTLSVerify(targetOptions.TlsVerify)
 
 			sourcePath := path.Join(outputDirectory, jobTarget.OsbuildArtifact.ExportName, jobTarget.OsbuildArtifact.ExportFilename)
 
diff --git a/internal/container/client.go b/internal/container/client.go
index c960d80af..8144d9520 100644
--- a/internal/container/client.go
+++ b/internal/container/client.go
@@ -12,6 +12,7 @@ import (
 	_ "github.com/containers/image/v5/docker/archive"
 	_ "github.com/containers/image/v5/oci/archive"
 	_ "github.com/containers/image/v5/oci/layout"
+	"github.com/osbuild/osbuild-composer/internal/common"
 
 	"github.com/containers/common/pkg/retry"
 	"github.com/containers/image/v5/copy"
@@ -48,7 +49,6 @@ type Client struct {
 	MaxRetries        int  // how often to retry http requests
 
 	UserAgent string // user agent string to use for requests, defaults to DefaultUserAgent
-	TlsVerify bool   // use an insecure connection
 
 	// internal state
 	policy *signature.Policy
@@ -89,7 +89,6 @@ func NewClient(target string) (*Client, error) {
 		PrecomputeDigests: true,
 
 		UserAgent: DefaultUserAgent,
-		TlsVerify: true,
 
 		sysCtx: &types.SystemContext{
 			RegistriesDirPath:        "",
@@ -108,6 +107,35 @@ func (cl *Client) SetCredentials(username, password string) {
 	cl.Auth.Password = password
 }
 
+// SetSkipTLSVerify controls if TLS verification happens when
+// making requests. If nil is passed it falls back to the default.
+func (cl *Client) SetTLSVerify(verify *bool) {
+	if verify == nil {
+		cl.sysCtx.DockerInsecureSkipTLSVerify = types.OptionalBoolUndefined
+	} else {
+		cl.sysCtx.DockerInsecureSkipTLSVerify = types.NewOptionalBool(!*verify)
+	}
+}
+
+// GetSkipTLSVerify returns current TLS verification state.
+func (cl *Client) GetTLSVerify() *bool {
+
+	skip := cl.sysCtx.DockerInsecureSkipTLSVerify
+
+	if skip == types.OptionalBoolUndefined {
+		return nil
+	}
+
+	// NB: we invert the state, i.e. verify == (skip == false)
+	return common.BoolToPtr(skip == types.OptionalBoolFalse)
+}
+
+// SkipTLSVerify is a convenience helper that internally calls
+// SetTLSVerify with false
+func (cl *Client) SkipTLSVerify() {
+	cl.SetTLSVerify(common.BoolToPtr(false))
+}
+
 func parseImageName(name string) (types.ImageReference, error) {
 
 	parts := strings.SplitN(name, ":", 2)
@@ -130,7 +158,6 @@ func parseImageName(name string) (types.ImageReference, error) {
 func (cl *Client) UploadImage(ctx context.Context, from, tag string) (digest.Digest, error) {
 
 	targetCtx := *cl.sysCtx
-	targetCtx.DockerInsecureSkipTLSVerify = types.NewOptionalBool(!cl.TlsVerify)
 	targetCtx.DockerRegistryPushPrecomputeDigests = cl.PrecomputeDigests
 
 	targetCtx.DockerAuthConfig = &types.DockerAuthConfig{