Reconfigure TLS functionality to use Listener

Fix a few manifest issues as well.

Signed-off-by: Tayler Geiger <tayler@redhat.com>

Makefile

@@ -48,7 +48,7 @@ clean: ## Remove binaries and test artifacts
 .PHONY: generate
 generate: $(CONTROLLER_GEN) ## Generate code and manifests.
 	$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."
-	$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
+	$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/base/crd/bases
 
 .PHONY: fmt
 fmt: ## Run go fmt against code.
@@ -156,12 +156,12 @@ install: build-container kind-load cert-manager deploy wait ## Install local cat
 
 .PHONY: deploy
 deploy: $(KUSTOMIZE) ## Deploy Catalogd to the K8s cluster specified in ~/.kube/config.
-	cd config/manager && $(KUSTOMIZE) edit set image controller=$(IMAGE)
-	$(KUSTOMIZE) build config/default | kubectl apply -f -
+	cd config/base/manager && $(KUSTOMIZE) edit set image controller=$(IMAGE)
+	$(KUSTOMIZE) build config/overlays/cert-manager | kubectl apply -f -
 
 .PHONY: undeploy
 undeploy: $(KUSTOMIZE) ## Undeploy Catalogd from the K8s cluster specified in ~/.kube/config.
-	$(KUSTOMIZE) build config/default | kubectl delete --ignore-not-found=true -f -
+	$(KUSTOMIZE) build config/overlays/cert-manager | kubectl delete --ignore-not-found=true -f -
 
 wait:
 	kubectl wait --for=condition=Available --namespace=$(CATALOGD_NAMESPACE) deployment/catalogd-controller-manager --timeout=60s
@@ -179,7 +179,7 @@ release: $(GORELEASER) ## Runs goreleaser for catalogd. By default, this will ru
 	$(GORELEASER) $(GORELEASER_ARGS)
 
 quickstart: $(KUSTOMIZE) generate ## Generate the installation release manifests and scripts
-	$(KUSTOMIZE) build config/default | sed "s/:devel/:$(GIT_VERSION)/g" > catalogd.yaml
+	$(KUSTOMIZE) build config/overlays/cert-manager | sed "s/:devel/:$(GIT_VERSION)/g" > catalogd.yaml
 
 .PHONY: demo-update
 demo-update:

Tiltfile

@@ -5,7 +5,7 @@ load('../tilt-support/Tiltfile', 'deploy_repo')
 
 repo = {
     'image': 'quay.io/operator-framework/catalogd',
-    'yaml': 'config/default',
+    'yaml': 'config/overlays/cert-manager',
     'binaries': {
         'manager': 'catalogd-controller-manager',
     },

api/core/v1alpha1/catalog_types_test.go

@@ -20,7 +20,7 @@ import (
 )
 
 func TestPollIntervalCELValidationRules(t *testing.T) {
-	validators := fieldValidatorsFromFile(t, "../../../config/crd/bases/catalogd.operatorframework.io_catalogs.yaml")
+	validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/catalogd.operatorframework.io_catalogs.yaml")
 	pth := "openAPIV3Schema.properties.spec"
 	validator, found := validators["v1alpha1"][pth]
 	assert.True(t, found)

cmd/manager/main.go

@@ -17,8 +17,10 @@ limitations under the License.
 package main
 
 import (
+	"crypto/tls"
 	"flag"
 	"fmt"
+	"net"
 	"net/http"
 	"net/url"
 	"os"
@@ -71,11 +73,12 @@ func main() {
 		catalogdVersion      bool
 		systemNamespace      string
 		catalogServerAddr    string
-		httpsExternalAddr    string
+		externalAddr         string
 		cacheDir             string
 		gcInterval           time.Duration
-		certfile             string
-		keyfile              string
+		certFile             string
+		keyFile              string
+		listener             net.Listener
 	)
 	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
@@ -85,12 +88,12 @@ func main() {
 			"Enabling this will ensure there is only one active controller manager.")
 	flag.StringVar(&systemNamespace, "system-namespace", "", "The namespace catalogd uses for internal state, configuration, and workloads")
 	flag.StringVar(&catalogServerAddr, "catalogs-server-addr", ":8083", "The address where the unpacked catalogs' content will be accessible")
-	flag.StringVar(&httpsExternalAddr, "https-external-address", "https://catalogd-catalogserver.catalogd-system.svc", "The external address at which the http server is reachable.")
+	flag.StringVar(&externalAddr, "external-address", "catalogd-catalogserver.catalogd-system.svc", "The external address at which the http(s) server is reachable.")
 	flag.StringVar(&cacheDir, "cache-dir", "/var/cache/", "The directory in the filesystem that catalogd will use for file based caching")
 	flag.BoolVar(&catalogdVersion, "version", false, "print the catalogd version and exit")
 	flag.DurationVar(&gcInterval, "gc-interval", 12*time.Hour, "interval in which garbage collection should be run against the catalog content cache")
-	flag.StringVar(&certfile, "tls-cert", "/var/certs/tls.crt", "The certificate file used for serving catalog contents over HTTPS")
-	flag.StringVar(&keyfile, "tls-key", "/var/certs/tls.key", "The key file used for serving catalog contents over HTTPS")
+	flag.StringVar(&certFile, "tls-cert", "", "The certificate file used for serving catalog contents over HTTPS")
+	flag.StringVar(&keyFile, "tls-key", "", "The key file used for serving catalog contents over HTTPS")
 	opts := zap.Options{
 		Development: true,
 	}
@@ -147,14 +150,40 @@ func main() {
 		os.Exit(1)
 	}
 
-	baseStorageURL, err := url.Parse(fmt.Sprintf("%s/catalogs/", httpsExternalAddr))
+	if certFile != "" && keyFile != "" {
+		cert, err := tls.LoadX509KeyPair(certFile, keyFile)
+		if err != nil {
+			setupLog.Error(err, "unable to load certificate key pair")
+			os.Exit(1)
+		}
+		config := &tls.Config{
+			Certificates: []tls.Certificate{cert},
+			MinVersion:   tls.VersionTLS13,
+		}
+		listener, err = tls.Listen("tcp", catalogServerAddr, config)
+		if err != nil {
+			setupLog.Error(err, "unable to create HTTPS server listener")
+			os.Exit(1)
+		}
+		externalAddr = "https://" + externalAddr
+	} else {
+		listener, err = net.Listen("tcp", catalogServerAddr)
+		if err != nil {
+			setupLog.Error(err, "unable to create HTTP server listener")
+			os.Exit(1)
+		}
+		externalAddr = "http://" + externalAddr
+	}
+
+	baseStorageURL, err := url.Parse(fmt.Sprintf("%s/catalogs/", externalAddr))
 	if err != nil {
 		setupLog.Error(err, "unable to create base storage URL")
 		os.Exit(1)
 	}
 
 	localStorage = storage.LocalDir{RootDir: storeDir, BaseURL: baseStorageURL}
 	shutdownTimeout := 30 * time.Second
+
 	catalogServer := server.Server{
 		Kind: "catalogs",
 		Server: &http.Server{
@@ -166,9 +195,7 @@ func main() {
 			WriteTimeout: 5 * time.Minute,
 		},
 		ShutdownTimeout: &shutdownTimeout,
-		ServeTLS:        true,
-		CertFile:        certfile,
-		KeyFile:         keyfile,
+		Listener:        listener,
 	}
 
 	if err := mgr.Add(&catalogServer); err != nil {

config/overlays/cert-manager/kustomization.yaml

@@ -1,5 +1,20 @@
+# Adds namespace to all resources.
+namespace: catalogd-system
+
+# Value of this field is prepended to the
+# names of all resources, e.g. a deployment named
+# "wordpress" becomes "alices-wordpress".
+# Note that it should also match with the prefix (text before '-') of the namespace
+# field above.
+namePrefix: catalogd-
+
+# the following config is for teaching kustomize how to do var substitution
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
 resources:
-- ../../base/default
+- ../../base/crd
+- ../../base/rbac
+- ../../base/manager
 - resources
 
 patches:

config/overlays/cert-manager/patches/manager_deployment_certs.yaml

@@ -4,4 +4,9 @@
 - op: add
   path: /spec/template/spec/containers/1/volumeMounts/-
   value: {"name":"catalogserver-certs", "mountPath":"/var/certs"}
-
+- op: add
+  path: /spec/template/spec/containers/1/args/-
+  value: "--tls-cert=/var/certs/tls.crt"
+- op: add
+  path: /spec/template/spec/containers/1/args/-
+  value: "--tls-key=/var/certs/tls.key"

config/overlays/cert-manager/resources/certificate.yaml

@@ -1,19 +1,14 @@
+---
 apiVersion: cert-manager.io/v1
 kind: Certificate
 metadata:
   name: catalogserver-cert
   namespace: system
 spec:
   secretName: catalogd-catalogserver-cert
-  duration: 2160h # 90d
-  renewBefore: 360h # 15d
-  subject:
-    organizations:
-      - operator-framework
-  isCA: false
   dnsNames:
+    - localhost
     - catalogd-catalogserver.catalogd-system.svc
   issuerRef:
-    name: catalogserver-selfsigned-issuer
     kind: Issuer
-
+    name: catalogd-catalogserver-ca-issuer

config/overlays/cert-manager/resources/issuer.yaml

@@ -1,7 +1,37 @@
+---
 apiVersion: cert-manager.io/v1
 kind: Issuer
 metadata:
   name: catalogserver-selfsigned-issuer
-  namespace: system
 spec:
   selfSigned: {}
+---
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: catalogserver-ca
+  namespace: system
+spec:
+  isCA: true
+  secretName: catalogd-catalogserver-ca
+  dnsNames:
+    - catalogd.io
+  duration: 2160h # 90d
+  renewBefore: 360h # 15d
+  privateKey:
+    rotationPolicy: Always
+    algorithm: ECDSA
+    size: 256
+  issuerRef:
+    name: catalogd-catalogserver-selfsigned-issuer
+    kind: Issuer
+    group: cert-manager.io
+---
+apiVersion: cert-manager.io/v1
+kind: Issuer
+metadata:
+  name: catalogserver-ca-issuer
+  namespace: system
+spec:
+  ca:
+    secretName: catalogd-catalogserver-ca

config/rbac/role.yaml

@@ -0,0 +1,65 @@
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: manager-role
+rules:
+- apiGroups:
+  - catalogd.operatorframework.io
+  resources:
+  - catalogs
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
+- apiGroups:
+  - catalogd.operatorframework.io
+  resources:
+  - catalogs/finalizers
+  verbs:
+  - update
+- apiGroups:
+  - catalogd.operatorframework.io
+  resources:
+  - catalogs/status
+  verbs:
+  - get
+  - patch
+  - update
+- apiGroups:
+  - ""
+  resources:
+  - pods
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
+- apiGroups:
+  - ""
+  resources:
+  - pods/log
+  verbs:
+  - get
+  - list
+  - watch
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: manager-role
+  namespace: system
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get

internal/third_party/server/server.go

@@ -38,7 +38,7 @@ var (
 	_ manager.LeaderElectionRunnable = (*Server)(nil)
 )
 
-// Server is a general purpose HTTP server Runnable for a manager.
+// Server is a general purpose HTTP(S) server Runnable for a manager.
 // It is used to serve some internal handlers for health probes and profiling,
 // but it can also be used to run custom servers.
 type Server struct {
@@ -62,18 +62,6 @@ type Server struct {
 	// ShutdownTimeout is an optional duration that indicates how long to wait for the server to shutdown gracefully. If not set,
 	// the server will wait indefinitely for all connections to close.
 	ShutdownTimeout *time.Duration
-
-	// ServeTLS is an optional bool that indicates that the server should
-	// serve over HTTPS
-	ServeTLS bool
-
-	// CertFile is the certificate file to use when serving over HTTPS.
-	// Only used and required when ServeTLS is "true".
-	CertFile string
-
-	// KeyFile is the key file to use when serving over HTTPS.
-	// Only used and required when ServeTLS is "true".
-	KeyFile string
 }
 
 // Start starts the server. It will block until the server is stopped or an error occurs.
@@ -128,15 +116,8 @@ func (s *Server) addr() string {
 
 func (s *Server) serve() error {
 	if s.Listener != nil {
-		if s.ServeTLS {
-			return s.Server.ServeTLS(s.Listener, s.CertFile, s.KeyFile)
-		}
 		return s.Server.Serve(s.Listener)
 	}
 
-	if s.ServeTLS {
-		return s.Server.ListenAndServeTLS(s.CertFile, s.KeyFile)
-	}
-
 	return s.Server.ListenAndServe()
 }

test/e2e/unpack_test.go

@@ -92,7 +92,7 @@ var _ = Describe("Catalog Unpacking", func() {
 			name := strings.Split(url.Hostname(), ".")[0]
 			port := url.Port()
 			// the ProxyGet() call below needs an explicit port value, so if
-			// value from url.Port() is empty, we assume port 80.
+			// value from url.Port() is empty, we assume port 443.
 			if port == "" {
 				port = "443"
 			}