Updating path for fetching 1password items to be of the op:// reference format

pkg/onepassword/annotations.go

@@ -9,7 +9,7 @@ import (
 
 const (
 	OnepasswordPrefix            = "operator.1password.io"
-	ItemPathAnnotation           = OnepasswordPrefix + "/item-path"
+	ItemReferenceAnnotation      = OnepasswordPrefix + "/item-reference"
 	NameAnnotation               = OnepasswordPrefix + "/item-name"
 	VersionAnnotation            = OnepasswordPrefix + "/item-version"
 	RestartAnnotation            = OnepasswordPrefix + "/last-restarted"

pkg/onepassword/annotations_test.go

@@ -22,7 +22,7 @@ func TestFilterAnnotations(t *testing.T) {
 	if len(filteredAnnotations) != 2 {
 		t.Errorf("Unexpected number of filtered annotations returned. Expected 2, got %v", len(filteredAnnotations))
 	}
-	_, found := filteredAnnotations[ItemPathAnnotation]
+	_, found := filteredAnnotations[ItemReferenceAnnotation]
 	if !found {
 		t.Errorf("One Password Annotation was filtered when it should not have been")
 	}
@@ -87,7 +87,7 @@ func TestGetNoAnnotationsForDeployment(t *testing.T) {
 
 func getValidAnnotations() map[string]string {
 	return map[string]string{
-		ItemPathAnnotation: "vaults/b3e4c7fc-8bf7-4c22-b8bb-147539f10e4f/items/b3e4c7fc-8bf7-4c22-b8bb-147539f10e4f",
-		NameAnnotation:     "secretName",
+		ItemReferenceAnnotation: "op://b3e4c7fc-8bf7-4c22-b8bb-147539f10e4f/b3e4c7fc-8bf7-4c22-b8bb-147539f10e4f",
+		NameAnnotation:          "secretName",
 	}
 }

pkg/onepassword/items.go

@@ -11,11 +11,16 @@ import (
 
 var logger = logf.Log.WithName("retrieve_item")
 
-func GetOnePasswordItemByPath(opConnectClient connect.Client, path string) (*onepassword.Item, error) {
-	vaultValue, itemValue, err := ParseVaultAndItemFromPath(path)
+const (
+	secretReferencePrefix = "op://"
+)
+
+func GetOnePasswordItemByReference(opConnectClient connect.Client, reference string) (*onepassword.Item, error) {
+	vaultValue, itemValue, err := ParseReference(reference)
 	if err != nil {
 		return nil, err
 	}
+
 	vaultId, err := getVaultId(opConnectClient, vaultValue)
 	if err != nil {
 		return nil, err
@@ -33,12 +38,28 @@ func GetOnePasswordItemByPath(opConnectClient connect.Client, path string) (*one
 	return item, nil
 }
 
-func ParseVaultAndItemFromPath(path string) (string, string, error) {
-	splitPath := strings.Split(path, "/")
-	if len(splitPath) == 4 && splitPath[0] == "vaults" && splitPath[2] == "items" {
-		return splitPath[1], splitPath[3], nil
+func ParseReference(reference string) (string, string, error) {
+	if !strings.HasPrefix(reference, secretReferencePrefix) {
+		return "", "", fmt.Errorf("secret reference should start with `op://`")
 	}
-	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)
+	path := strings.TrimPrefix(reference, secretReferencePrefix)
+
+	splitPath := strings.Split(path, "/")
+	if len(splitPath) != 2 {
+		return "", "", fmt.Errorf("Invalid secret reference : %s. Secret references should match op://<vault>/<item>", reference)
+	}
+
+	vault := splitPath[0]
+	if vault == "" {
+		return "", "", fmt.Errorf("Invalid secret reference : %s. Vault can't be empty.", reference)
+	}
+
+	item := splitPath[1]
+	if item == "" {
+		return "", "", fmt.Errorf("Invalid secret reference : %s. Item can't be empty.", reference)
+	}
+
+	return vault, item, nil
 }
 
 func getVaultId(client connect.Client, vaultIdentifier string) (string, error) {

pkg/onepassword/secret_update_handler.go

@@ -110,13 +110,13 @@ func (h *SecretUpdateHandler) updateKubernetesSecrets() (map[string]map[string]*
 	for i := 0; i < len(secrets.Items); i++ {
 		secret := secrets.Items[i]
 
-		itemPath := secret.Annotations[ItemPathAnnotation]
+		itemReference := secret.Annotations[ItemReferenceAnnotation]
 		currentVersion := secret.Annotations[VersionAnnotation]
-		if len(itemPath) == 0 || len(currentVersion) == 0 {
+		if len(itemReference) == 0 || len(currentVersion) == 0 {
 			continue
 		}
 
-		item, err := GetOnePasswordItemByPath(h.opConnectClient, secret.Annotations[ItemPathAnnotation])
+		item, err := GetOnePasswordItemByReference(h.opConnectClient, secret.Annotations[ItemReferenceAnnotation])
 		if err != nil {
 			return nil, fmt.Errorf("Failed to retrieve item: %v", err)
 		}

pkg/onepassword/secret_update_handler_test.go

@@ -51,7 +51,7 @@ var (
 		"password": []byte(password),
 		"username": []byte(username),
 	}
-	itemPath = fmt.Sprintf("vaults/%v/items/%v", vaultId, itemId)
+	itemReference = fmt.Sprintf("op://%v/%v", vaultId, itemId)
 )
 
 var defaultNamespace = &corev1.Namespace{
@@ -73,8 +73,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					NameAnnotation:     "unlrelated secret",
-					ItemPathAnnotation: itemPath,
+					NameAnnotation:          "unlrelated secret",
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 		},
@@ -83,8 +83,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  "old version",
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       "old version",
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -95,8 +95,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  fmt.Sprint(itemVersion),
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       fmt.Sprint(itemVersion),
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -149,8 +149,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  "old version",
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       "old version",
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -161,8 +161,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  fmt.Sprint(itemVersion),
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       fmt.Sprint(itemVersion),
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -186,8 +186,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					ItemPathAnnotation: itemPath,
-					NameAnnotation:     name,
+					ItemReferenceAnnotation: itemReference,
+					NameAnnotation:          name,
 				},
 			},
 		},
@@ -196,8 +196,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  "old version",
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       "old version",
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -208,8 +208,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  fmt.Sprint(itemVersion),
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       fmt.Sprint(itemVersion),
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -255,8 +255,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  "old version",
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       "old version",
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -267,8 +267,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  fmt.Sprint(itemVersion),
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       fmt.Sprint(itemVersion),
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -292,8 +292,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					ItemPathAnnotation: itemPath,
-					NameAnnotation:     name,
+					ItemReferenceAnnotation: itemReference,
+					NameAnnotation:          name,
 				},
 			},
 		},
@@ -302,8 +302,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  fmt.Sprint(itemVersion),
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       fmt.Sprint(itemVersion),
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -314,8 +314,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  fmt.Sprint(itemVersion),
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       fmt.Sprint(itemVersion),
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -369,8 +369,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  "old version",
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       "old version",
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -381,8 +381,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  fmt.Sprint(itemVersion),
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       fmt.Sprint(itemVersion),
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -439,7 +439,7 @@ var tests = []testUpdateSecretTask{
 				Namespace: namespace,
 				Annotations: map[string]string{
 					VersionAnnotation:            "old version",
-					ItemPathAnnotation:           itemPath,
+					ItemReferenceAnnotation:      itemReference,
 					RestartDeploymentsAnnotation: "true",
 				},
 			},
@@ -452,7 +452,7 @@ var tests = []testUpdateSecretTask{
 				Namespace: namespace,
 				Annotations: map[string]string{
 					VersionAnnotation:            fmt.Sprint(itemVersion),
-					ItemPathAnnotation:           itemPath,
+					ItemReferenceAnnotation:      itemReference,
 					RestartDeploymentsAnnotation: "true",
 				},
 			},
@@ -510,7 +510,7 @@ var tests = []testUpdateSecretTask{
 				Namespace: namespace,
 				Annotations: map[string]string{
 					VersionAnnotation:            "old version",
-					ItemPathAnnotation:           itemPath,
+					ItemReferenceAnnotation:      itemReference,
 					RestartDeploymentsAnnotation: "false",
 				},
 			},
@@ -523,7 +523,7 @@ var tests = []testUpdateSecretTask{
 				Namespace: namespace,
 				Annotations: map[string]string{
 					VersionAnnotation:            fmt.Sprint(itemVersion),
-					ItemPathAnnotation:           itemPath,
+					ItemReferenceAnnotation:      itemReference,
 					RestartDeploymentsAnnotation: "false",
 				},
 			},
@@ -580,8 +580,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  "old version",
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       "old version",
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -592,8 +592,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  fmt.Sprint(itemVersion),
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       fmt.Sprint(itemVersion),
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -657,8 +657,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  "old version",
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       "old version",
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -669,8 +669,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  fmt.Sprint(itemVersion),
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       fmt.Sprint(itemVersion),
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -730,8 +730,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  "old version",
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       "old version",
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,
@@ -742,8 +742,8 @@ var tests = []testUpdateSecretTask{
 				Name:      name,
 				Namespace: namespace,
 				Annotations: map[string]string{
-					VersionAnnotation:  fmt.Sprint(itemVersion),
-					ItemPathAnnotation: itemPath,
+					VersionAnnotation:       fmt.Sprint(itemVersion),
+					ItemReferenceAnnotation: itemReference,
 				},
 			},
 			Data: expectedSecretData,