Merge pull request #64 from 1Password/release/v1.0.2

Preparing release

.VERSION

@@ -1 +1 @@
-v1.0.1
+1.0.2

CHANGELOG.md

@@ -12,6 +12,14 @@
 
 ---
 
+[//]: # (START/v1.0.2)
+# v1.0.2
+
+## Fixes
+ * Name normalizer added to handle non-conforming item names.
+
+---
+
 [//]: # (START/v1.0.1)
 # v1.0.1
 

Makefile

@@ -20,12 +20,12 @@ test/coverage:	## Run test suite with coverage report
 	go test -v ./... -cover
 
 build:	## Build operator Docker image
-	@docker build -f Dockerfile --build-arg operator_version=$(curVersion) -t $(DOCKER_IMG_TAG)
+	@docker build -f Dockerfile --build-arg operator_version=$(curVersion) -t $(DOCKER_IMG_TAG) .
 	@echo "Successfully built and tagged image."
 	@echo "Tag: $(DOCKER_IMG_TAG)"
 
 build/local:	## Build local version of the operator Docker image
-	@docker build -f Dockerfile -t local/$(DOCKER_IMG_TAG)
+	@docker build -f Dockerfile -t local/$(DOCKER_IMG_TAG) .
 
 build/binary: clean	## Build operator binary
 	@mkdir -p dist

README.md

@@ -84,9 +84,9 @@ An sample Deployment yaml can be found at `/deploy/operator.yaml`.
 
 To further configure the 1Password Kubernetes Operator the Following Environment variables can be set in the operator yaml:
 
-- **WATCH_NAMESPACE:** comma separated list of what Namespaces to watch for changes.
 - **OP_CONNECT_HOST** (required): Specifies the host name within Kubernetes in which to access the 1Password Connect.
-- **POLLING_INTERVAL** (default: 600)**:** The number of seconds the 1Password Kubernetes Operator will wait before checking for updates from 1Password Connect.
+- **WATCH_NAMESPACE:** (default: watch all namespaces): Comma separated list of what Namespaces to watch for changes.
+- **POLLING_INTERVAL** (default: 600): The number of seconds the 1Password Kubernetes Operator will wait before checking for updates from 1Password Connect.
 - **MANAGE_CONNECT** (default: false): If set to true, on deployment of the operator, a default configuration of the OnePassword Connect Service will be deployed to the `default` namespace.
 - **AUTO_RESTART** (default: false): If set to true, the operator will restart any deployment using a secret from 1Password Connect. This can be overwritten by namespace, deployment, or individual secret. More details on AUTO_RESTART can be found in the ["Configuring Automatic Rolling Restarts of Deployments"](#configuring-automatic-rolling-restarts-of-deployments) section.
 
@@ -144,7 +144,12 @@ If a 1Password Item that is linked to a Kubernetes Secret is updated within the
 ---
 **NOTE**
 
-If multiple 1Password vaults/items have the same `title` when using a title in the access path, the desired action will be performed on the oldest vault/item. Furthermore, titles that include white space characters cannot be used.
+If multiple 1Password vaults/items have the same `title` when using a title in the access path, the desired action will be performed on the oldest vault/item. 
+
+Titles and field names that include white space and other characters that are not a valid [DNS subdomain name](https://kubernetes.io/docs/concepts/configuration/secret/) will create Kubernetes secrets that have titles and fields in the following format:
+ - Invalid characters before the first alphanumeric character and after the last alphanumeric character will be removed
+ - All whitespaces between words will be replaced by `-`
+ - All the letters will be lower-cased.
 
 ---
 

cmd/manager/main.go

@@ -83,9 +83,11 @@ func main() {
 
 	printVersion()
 
-	namespace, err := k8sutil.GetWatchNamespace()
+	namespace := os.Getenv(k8sutil.WatchNamespaceEnvVar)
+
+	deploymentNamespace, err := k8sutil.GetOperatorNamespace()
 	if err != nil {
-		log.Error(err, "Failed to get watch namespace")
+		log.Error(err, "Failed to get namespace")
 		os.Exit(1)
 	}
 
@@ -139,7 +141,7 @@ func main() {
 		go func() {
 			connectStarted := false
 			for connectStarted == false {
-				err := op.SetupConnect(mgr.GetClient())
+				err := op.SetupConnect(mgr.GetClient(), deploymentNamespace)
 				// Cache Not Started is an acceptable error. Retry until cache is started.
 				if err != nil && !errors.Is(err, &cache.ErrCacheNotStarted{}) {
 					log.Error(err, "")

deploy/connect/deployment.yaml

@@ -2,7 +2,6 @@ apiVersion: apps/v1
 kind: Deployment
 metadata:
   name: onepassword-connect
-  namespace: default
 spec:
   selector:
     matchLabels:

deploy/connect/service.yaml

@@ -2,7 +2,6 @@ apiVersion: v1
 kind: Service
 metadata:
   name: onepassword-connect
-  namespace: default
 spec:
   type: NodePort
   selector:

pkg/controller/onepassworditem/onepassworditem_controller.go

@@ -3,7 +3,6 @@ package onepassworditem
 import (
 	"context"
 	"fmt"
-
 	onepasswordv1 "github.com/1Password/onepassword-operator/pkg/apis/onepassword/v1"
 	kubeSecrets "github.com/1Password/onepassword-operator/pkg/kubernetessecrets"
 	"github.com/1Password/onepassword-operator/pkg/onepassword"

pkg/controller/onepassworditem/onepassworditem_test.go

@@ -31,6 +31,9 @@ const (
 	itemId                    = "nwrhuano7bcwddcviubpp4mhfq"
 	username                  = "test-user"
 	password                  = "QmHumKc$mUeEem7caHtbaBaJ"
+	firstHost                 = "http://localhost:8080"
+	awsKey                    = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"
+	iceCream                  = "freezing blue 20%"
 	userKey                   = "username"
 	passKey                   = "password"
 	version                   = 123
@@ -210,6 +213,79 @@ var tests = []testReconcileItem{
 			passKey: password,
 		},
 	},
+	{
+		testName: "Secret from 1Password item with invalid K8s labels",
+		customResource: &onepasswordv1.OnePasswordItem{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       onePasswordItemKind,
+				APIVersion: onePasswordItemAPIVersion,
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "!my sECReT it3m%",
+				Namespace: namespace,
+			},
+			Spec: onepasswordv1.OnePasswordItemSpec{
+				ItemPath: itemPath,
+			},
+		},
+		existingSecret: nil,
+		expectedError:  nil,
+		expectedResultSecret: &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "my-secret-it3m",
+				Namespace: namespace,
+				Annotations: map[string]string{
+					op.VersionAnnotation: fmt.Sprint(version),
+				},
+			},
+			Data: expectedSecretData,
+		},
+		opItem: map[string]string{
+			userKey: username,
+			passKey: password,
+		},
+	},
+	{
+		testName: "Secret from 1Password item with fields and sections that have invalid K8s labels",
+		customResource: &onepasswordv1.OnePasswordItem{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       onePasswordItemKind,
+				APIVersion: onePasswordItemAPIVersion,
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "!my sECReT it3m%",
+				Namespace: namespace,
+			},
+			Spec: onepasswordv1.OnePasswordItemSpec{
+				ItemPath: itemPath,
+			},
+		},
+		existingSecret: nil,
+		expectedError:  nil,
+		expectedResultSecret: &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "my-secret-it3m",
+				Namespace: namespace,
+				Annotations: map[string]string{
+					op.VersionAnnotation: fmt.Sprint(version),
+				},
+			},
+			Data: map[string][]byte{
+				"password":       []byte(password),
+				"username":       []byte(username),
+				"first-host":     []byte(firstHost),
+				"aws-access-key": []byte(awsKey),
+				"ice-cream-type": []byte(iceCream),
+			},
+		},
+		opItem: map[string]string{
+			userKey:            username,
+			passKey:            password,
+			"first host":       firstHost,
+			"AWS Access Key":   awsKey,
+			"😄 ice-cream type": iceCream,
+		},
+	},
 }
 
 func TestReconcileOnePasswordItem(t *testing.T) {
@@ -241,7 +317,10 @@ func TestReconcileOnePasswordItem(t *testing.T) {
 			mocks.GetGetItemFunc = func(uuid string, vaultUUID string) (*onepassword.Item, error) {
 
 				item := onepassword.Item{}
-				item.Fields = generateFields(testData.opItem["username"], testData.opItem["password"])
+				item.Fields = []*onepassword.ItemField{}
+				for k, v := range testData.opItem {
+					item.Fields = append(item.Fields, &onepassword.ItemField{Label: k, Value: v})
+				}
 				item.Version = version
 				item.Vault.ID = vaultUUID
 				item.ID = uuid
@@ -257,8 +336,8 @@ func TestReconcileOnePasswordItem(t *testing.T) {
 			// watched resource .
 			req := reconcile.Request{
 				NamespacedName: types.NamespacedName{
-					Name:      name,
-					Namespace: namespace,
+					Name:      testData.customResource.ObjectMeta.Name,
+					Namespace: testData.customResource.ObjectMeta.Namespace,
 				},
 			}
 			_, err := r.Reconcile(req)

pkg/kubernetessecrets/kubernetes_secrets_builder.go

@@ -4,12 +4,17 @@ import (
 	"context"
 	"fmt"
 
+	"regexp"
+	"strings"
+
 	"github.com/1Password/connect-sdk-go/onepassword"
 	"github.com/1Password/onepassword-operator/pkg/utils"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	kubeValidate "k8s.io/apimachinery/pkg/util/validation"
+
 	kubernetesClient "sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
@@ -63,7 +68,7 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa
 func BuildKubernetesSecretFromOnePasswordItem(name, namespace string, annotations map[string]string, item onepassword.Item) *corev1.Secret {
 	return &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        name,
+			Name:        formatSecretName(name),
 			Namespace:   namespace,
 			Annotations: annotations,
 		},
@@ -75,8 +80,33 @@ func BuildKubernetesSecretData(fields []*onepassword.ItemField) map[string][]byt
 	secretData := map[string][]byte{}
 	for i := 0; i < len(fields); i++ {
 		if fields[i].Value != "" {
-			secretData[fields[i].Label] = []byte(fields[i].Value)
+			key := formatSecretName(fields[i].Label)
+			secretData[key] = []byte(fields[i].Value)
 		}
 	}
 	return secretData
 }
+
+// formatSecretName rewrites a value to be a valid Secret name or Secret data key.
+//
+// The Secret meta.name and data keys must be valid DNS subdomain names (https://kubernetes.io/docs/concepts/configuration/secret/#overview-of-secrets)
+func formatSecretName(value string) string {
+	if errs := kubeValidate.IsDNS1123Subdomain(value); len(errs) == 0 {
+		return value
+	}
+	return createValidSecretName(value)
+}
+
+var invalidDNS1123Chars = regexp.MustCompile("[^a-z0-9-]+")
+
+func createValidSecretName(value string) string {
+	result := strings.ToLower(value)
+	result = invalidDNS1123Chars.ReplaceAllString(result, "-")
+
+	if len(result) > kubeValidate.DNS1123SubdomainMaxLength {
+		result = result[0:kubeValidate.DNS1123SubdomainMaxLength]
+	}
+
+	// first and last character MUST be alphanumeric
+	return strings.Trim(result, "-")
+}

pkg/kubernetessecrets/kubernetes_secrets_builder_test.go

@@ -3,6 +3,7 @@ package kubernetessecrets
 import (
 	"context"
 	"fmt"
+	kubeValidate "k8s.io/apimachinery/pkg/util/validation"
 	"strings"
 	"testing"
 
@@ -101,7 +102,7 @@ func TestBuildKubernetesSecretFromOnePasswordItem(t *testing.T) {
 	item.Fields = generateFields(5)
 
 	kubeSecret := BuildKubernetesSecretFromOnePasswordItem(name, namespace, annotations, item)
-	if kubeSecret.Name != name {
+	if kubeSecret.Name != strings.ToLower(name) {
 		t.Errorf("Expected name value: %v but got: %v", name, kubeSecret.Name)
 	}
 	if kubeSecret.Namespace != namespace {
@@ -113,6 +114,44 @@ func TestBuildKubernetesSecretFromOnePasswordItem(t *testing.T) {
 	compareFields(item.Fields, kubeSecret.Data, t)
 }
 
+func TestBuildKubernetesSecretFixesInvalidLabels(t *testing.T) {
+	name := "inV@l1d k8s secret%name"
+	expectedName := "inv-l1d-k8s-secret-name"
+	namespace := "someNamespace"
+	annotations := map[string]string{
+		"annotationKey": "annotationValue",
+	}
+	item := onepassword.Item{}
+
+	item.Fields = []*onepassword.ItemField{
+		{
+			Label: "label w%th invalid ch!rs-",
+			Value: "value1",
+		},
+		{
+			Label: strings.Repeat("x", kubeValidate.DNS1123SubdomainMaxLength+1),
+			Value: "name exceeds max length",
+		},
+	}
+
+	kubeSecret := BuildKubernetesSecretFromOnePasswordItem(name, namespace, annotations, item)
+
+	// Assert Secret's meta.name was fixed
+	if kubeSecret.Name != expectedName {
+		t.Errorf("Expected name value: %v but got: %v", name, kubeSecret.Name)
+	}
+	if kubeSecret.Namespace != namespace {
+		t.Errorf("Expected namespace value: %v but got: %v", namespace, kubeSecret.Namespace)
+	}
+
+	// assert labels were fixed for each data key
+	for key := range kubeSecret.Data {
+		if !validLabel(key) {
+			t.Errorf("Expected valid kubernetes label, got %s", key)
+		}
+	}
+}
+
 func compareAnnotationsToItem(annotations map[string]string, item onepassword.Item, t *testing.T) {
 	actualVaultId, actualItemId, err := ParseVaultIdAndItemIdFromPath(annotations[ItemPathAnnotation])
 	if err != nil {
@@ -164,3 +203,10 @@ func ParseVaultIdAndItemIdFromPath(path string) (string, string, error) {
 	}
 	return "", "", fmt.Errorf("%q is not an acceptable path for One Password item. Must be of the format: `vaults/{vault_id}/items/{item_id}`", path)
 }
+
+func validLabel(v string) bool {
+	if err := kubeValidate.IsDNS1123Subdomain(v); len(err) > 0 {
+		return false
+	}
+	return true
+}

pkg/onepassword/connect_setup.go

@@ -2,6 +2,7 @@ package onepassword
 
 import (
 	"context"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"os"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -17,13 +18,13 @@ var logConnectSetup = logf.Log.WithName("ConnectSetup")
 var deploymentPath = "deploy/connect/deployment.yaml"
 var servicePath = "deploy/connect/service.yaml"
 
-func SetupConnect(kubeClient client.Client) error {
-	err := setupService(kubeClient, servicePath)
+func SetupConnect(kubeClient client.Client, deploymentNamespace string) error {
+	err := setupService(kubeClient, servicePath, deploymentNamespace)
 	if err != nil {
 		return err
 	}
 
-	err = setupDeployment(kubeClient, deploymentPath)
+	err = setupDeployment(kubeClient, deploymentPath, deploymentNamespace)
 	if err != nil {
 		return err
 	}
@@ -31,22 +32,22 @@ func SetupConnect(kubeClient client.Client) error {
 	return nil
 }
 
-func setupDeployment(kubeClient client.Client, deploymentPath string) error {
+func setupDeployment(kubeClient client.Client, deploymentPath string, deploymentNamespace string) error {
 	existingDeployment := &appsv1.Deployment{}
 
 	// check if deployment has already been created
-	err := kubeClient.Get(context.Background(), types.NamespacedName{Name: "onepassword-connect", Namespace: "default"}, existingDeployment)
+	err := kubeClient.Get(context.Background(), types.NamespacedName{Name: "onepassword-connect", Namespace: deploymentNamespace}, existingDeployment)
 	if err != nil {
 		if errors.IsNotFound(err) {
 			logConnectSetup.Info("No existing Connect deployment found. Creating Deployment")
-			return createDeployment(kubeClient, deploymentPath)
+			return createDeployment(kubeClient, deploymentPath, deploymentNamespace)
 		}
 	}
 	return err
 }
 
-func createDeployment(kubeClient client.Client, deploymentPath string) error {
-	deployment, err := getDeploymentToCreate(deploymentPath)
+func createDeployment(kubeClient client.Client, deploymentPath string, deploymentNamespace string) error {
+	deployment, err := getDeploymentToCreate(deploymentPath, deploymentNamespace)
 	if err != nil {
 		return err
 	}
@@ -59,12 +60,16 @@ func createDeployment(kubeClient client.Client, deploymentPath string) error {
 	return nil
 }
 
-func getDeploymentToCreate(deploymentPath string) (*appsv1.Deployment, error) {
+func getDeploymentToCreate(deploymentPath string, deploymentNamespace string) (*appsv1.Deployment, error) {
 	f, err := os.Open(deploymentPath)
 	if err != nil {
 		return nil, err
 	}
-	deployment := &appsv1.Deployment{}
+	deployment := &appsv1.Deployment{
+		ObjectMeta: v1.ObjectMeta{
+			Namespace: deploymentNamespace,
+		},
+	}
 
 	err = yaml.NewYAMLOrJSONDecoder(f, 4096).Decode(deployment)
 	if err != nil {
@@ -73,26 +78,30 @@ func getDeploymentToCreate(deploymentPath string) (*appsv1.Deployment, error) {
 	return deployment, nil
 }
 
-func setupService(kubeClient client.Client, servicePath string) error {
+func setupService(kubeClient client.Client, servicePath string, deploymentNamespace string) error {
 	existingService := &corev1.Service{}
 
 	//check if service has already been created
-	err := kubeClient.Get(context.Background(), types.NamespacedName{Name: "onepassword-connect", Namespace: "default"}, existingService)
+	err := kubeClient.Get(context.Background(), types.NamespacedName{Name: "onepassword-connect", Namespace: deploymentNamespace}, existingService)
 	if err != nil {
 		if errors.IsNotFound(err) {
 			logConnectSetup.Info("No existing Connect service found. Creating Service")
-			return createService(kubeClient, servicePath)
+			return createService(kubeClient, servicePath, deploymentNamespace)
 		}
 	}
 	return err
 }
 
-func createService(kubeClient client.Client, servicePath string) error {
+func createService(kubeClient client.Client, servicePath string, deploymentNamespace string) error {
 	f, err := os.Open(servicePath)
 	if err != nil {
 		return err
 	}
-	service := &corev1.Service{}
+	service := &corev1.Service{
+		ObjectMeta: v1.ObjectMeta{
+			Namespace: deploymentNamespace,
+		},
+	}
 
 	err = yaml.NewYAMLOrJSONDecoder(f, 4096).Decode(service)
 	if err != nil {

pkg/onepassword/connect_setup_test.go

@@ -25,7 +25,7 @@ func TestServiceSetup(t *testing.T) {
 	// Create a fake client to mock API calls.
 	client := fake.NewFakeClientWithScheme(s, objs...)
 
-	err := setupService(client, "../../deploy/connect/service.yaml")
+	err := setupService(client, "../../deploy/connect/service.yaml", defaultNamespacedName.Namespace)
 
 	if err != nil {
 		t.Errorf("Error Setting Up Connect: %v", err)
@@ -50,7 +50,7 @@ func TestDeploymentSetup(t *testing.T) {
 	// Create a fake client to mock API calls.
 	client := fake.NewFakeClientWithScheme(s, objs...)
 
-	err := setupDeployment(client, "../../deploy/connect/deployment.yaml")
+	err := setupDeployment(client, "../../deploy/connect/deployment.yaml", defaultNamespacedName.Namespace)
 
 	if err != nil {
 		t.Errorf("Error Setting Up Connect: %v", err)