Browse Source

feat(onepasswordSDK): multi-field and complete PushSecret (#6187)

Dariusch Ochlast 5 days ago
parent
commit
57b73730e9

+ 8 - 1
docs/provider/1password-sdk.md

@@ -67,7 +67,8 @@ kind: Secret
 metadata:
   name: source-secret
 stringData:
-  source-key: "my-secret"
+  api-key: "my-api-key"
+  api-url: "https://example.com/api"
 ```
 
 Looks like this:
@@ -79,6 +80,12 @@ Looks like this:
 Once all fields of a secret are deleted, the entire secret is deleted if the PushSecret object is removed and
 policy is set to `delete`.
 
+To sync the entire secret into a single 1Password item, the following configuration can be used:
+
+```yaml
+{% include '1passwordsdk-push-secret-all-keys.yaml' %}
+```
+
 ### Supported Functionality
 
 Please check the documentation on 1password for [Supported Functionality](https://developer.1password.com/docs/sdks/functionality).

+ 23 - 0
docs/snippets/1passwordsdk-push-secret-all-keys.yaml

@@ -0,0 +1,23 @@
+---
+apiVersion: external-secrets.io/v1alpha1
+kind: PushSecret
+metadata:
+  name: pushsecret-all-keys-example # customizable
+spec:
+  deletionPolicy: Delete
+  refreshInterval: 1h0m0s
+  secretStoreRefs:
+    - name: onepassword
+      kind: SecretStore
+  selector:
+    secret:
+      name: source-secret # Source Kubernetes secret
+  data:
+    - match:
+        remoteRef:
+          remoteKey: 1pw-item-name-all-keys # 1Password item name, each Kubernetes secret key becomes a separate concealed field
+      metadata:
+        apiVersion: kubernetes.external-secrets.io/v1alpha1
+        kind: PushSecretMetadata
+        spec:
+          tags: ["tag1", "tag2"]  # (Optional) tags on the 1Password item

+ 18 - 5
docs/snippets/1passwordsdk-push-secret.yaml

@@ -2,7 +2,7 @@
 apiVersion: external-secrets.io/v1alpha1
 kind: PushSecret
 metadata:
-  name: pushsecret-example # Customisable
+  name: pushsecret-example # customizable
 spec:
   deletionPolicy: Delete
   refreshInterval: 1h0m0s
@@ -14,12 +14,25 @@ spec:
       name: source-secret # Source Kubernetes secret
   data:
     - match:
-        secretKey: source-key # Source Kubernetes secret key to be pushed
+        secretKey: api-key # Source Kubernetes secret key to be pushed
         remoteRef:
-          remoteKey: 1pw-secret-name # 1Password item/secret name
-          property: password         # (Optional) 1Password field type, default password
+          remoteKey: 1pw-item-name     # 1Password item name
+          property: password           # Field label within the 1Password item
       metadata:
         apiVersion: kubernetes.external-secrets.io/v1alpha1
         kind: PushSecretMetadata
         spec:
-          tags: ["tag1", "tag2"]    # Optional metadata to be pushed with the secret
+          tags: ["tag1", "tag2"]  # (Optional) tags on the 1Password item (item-level, not field-level)
+                                  # Tags are shared across all fields of the same remoteKey — last write wins if entries differ
+          fieldType: concealed    # (Optional) field type (default: concealed)
+                                  # Accepted values (case-insensitive): text|string|concealed|password|url|email|phone|date|monthYear
+    - match:
+        secretKey: api-url
+        remoteRef:
+          remoteKey: 1pw-item-name     # Same 1Password item — adds a second field
+          property: api-endpoint
+      metadata:
+        apiVersion: kubernetes.external-secrets.io/v1alpha1
+        kind: PushSecretMetadata
+        spec:
+          fieldType: url

+ 178 - 30
providers/v1/onepasswordsdk/client.go

@@ -42,6 +42,11 @@ const (
 	errExpectedOneFieldMsgF = "found more than 1 fields with title '%s' in '%s', got %d"
 	itemCachePrefix         = "item:"
 	fileCachePrefix         = "file:"
+	defaultFieldLabel       = "password"
+
+	errMsgUpdateItem    = "failed to update item: %w"
+	errMsgCreateItem    = "failed to create item: %w"
+	errMsgParsePushMeta = "failed to parse push secret metadata: %w"
 )
 
 // ErrKeyNotFound is returned when a key is not found in the 1Password Vaults.
@@ -59,7 +64,8 @@ func isNativeItemID(s string) bool {
 
 // PushSecretMetadataSpec defines the metadata configuration for pushing secrets to 1Password.
 type PushSecretMetadataSpec struct {
-	Tags []string `json:"tags,omitempty"`
+	Tags      []string `json:"tags,omitempty"`
+	FieldType string   `json:"fieldType,omitempty"`
 }
 
 // GetSecret returns a single secret from 1Password provider.
@@ -101,6 +107,8 @@ func (p *SecretsClient) DeleteSecret(ctx context.Context, ref esv1.PushSecretRem
 		return err
 	}
 
+	providerItem.Fields = normalizeItemFields(providerItem.Fields)
+
 	var deleted bool
 	providerItem.Fields, deleted, err = deleteField(providerItem.Fields, ref.GetProperty())
 	if err != nil {
@@ -135,7 +143,7 @@ func (p *SecretsClient) DeleteSecret(ctx context.Context, ref esv1.PushSecretRem
 	_, err = p.client.Items().Put(ctx, providerItem)
 	metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKItemsPut, err)
 	if err != nil {
-		return fmt.Errorf("failed to update item: %w", err)
+		return fmt.Errorf(errMsgUpdateItem, err)
 	}
 
 	p.invalidateCacheByPrefix(p.constructRefKey(ref.GetRemoteKey()))
@@ -283,12 +291,12 @@ func getObjType(documentType onepassword.ItemCategory, property string) (string,
 func (p *SecretsClient) createItem(ctx context.Context, val []byte, ref esv1.PushSecretData) error {
 	mdata, err := metadata.ParseMetadataParameters[PushSecretMetadataSpec](ref.GetMetadata())
 	if err != nil {
-		return fmt.Errorf("failed to parse push secret metadata: %w", err)
+		return fmt.Errorf(errMsgParsePushMeta, err)
 	}
 
 	label := ref.GetProperty()
 	if label == "" {
-		label = "password"
+		label = defaultFieldLabel
 	}
 
 	var tags []string
@@ -296,18 +304,23 @@ func (p *SecretsClient) createItem(ctx context.Context, val []byte, ref esv1.Pus
 		tags = mdata.Spec.Tags
 	}
 
+	fieldType := onepassword.ItemFieldTypeConcealed
+	if mdata != nil {
+		fieldType = resolveFieldType(mdata.Spec.FieldType)
+	}
+
 	_, err = p.client.Items().Create(ctx, onepassword.ItemCreateParams{
 		Category: onepassword.ItemCategoryServer,
 		VaultID:  p.vaultID,
 		Title:    ref.GetRemoteKey(),
 		Fields: []onepassword.ItemField{
-			generateNewItemField(label, string(val)),
+			generateNewItemField(label, string(val), fieldType),
 		},
 		Tags: tags,
 	})
 	metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKItemsCreate, err)
 	if err != nil {
-		return fmt.Errorf("failed to create item: %w", err)
+		return fmt.Errorf(errMsgCreateItem, err)
 	}
 
 	p.invalidateCacheByPrefix(p.constructRefKey(ref.GetRemoteKey()))
@@ -317,10 +330,10 @@ func (p *SecretsClient) createItem(ctx context.Context, val []byte, ref esv1.Pus
 }
 
 // updateFieldValue updates the fields value of an item with the given label.
-// If the label does not exist, a new field is created. If the label exists but
+// If the label does not exist, a new field is created with the given fieldType. If the label exists but
 // the value is different, the value is updated. If the label exists and the
 // value is the same, nothing is done.
-func updateFieldValue(fields []onepassword.ItemField, title, newVal string) ([]onepassword.ItemField, error) {
+func updateFieldValue(fields []onepassword.ItemField, title, newVal string, fieldType onepassword.ItemFieldType) ([]onepassword.ItemField, error) {
 	// This will always iterate over all items.
 	// This is done to ensure that two fields with the same label
 	// exist resulting in undefined behavior.
@@ -338,29 +351,69 @@ func updateFieldValue(fields []onepassword.ItemField, title, newVal string) ([]o
 		}
 	}
 	if !found {
-		return append(fields, generateNewItemField(title, newVal)), nil
+		return append(fields, generateNewItemField(title, newVal, fieldType)), nil
 	}
 
 	if fields[index].Value != newVal {
 		fields[index].Value = newVal
 	}
+	if fields[index].FieldType != fieldType {
+		fields[index].FieldType = fieldType
+	}
 
 	return fields, nil
 }
 
-// generateNewItemField generates a new item field with the given label and value.
-func generateNewItemField(title, newVal string) onepassword.ItemField {
-	field := onepassword.ItemField{
+// resolveFieldType maps a 1Password field type name to the SDK constant.
+// Case-insensitive. Accepted: text|string, concealed|password, url, email, phone, date, monthYear.
+// Defaults to Concealed for empty/unrecognized. OTP and file excluded.
+// Reference: https://developer.1password.com/docs/cli/item-fields/#custom-fields
+func resolveFieldType(raw string) onepassword.ItemFieldType {
+	switch strings.ToLower(raw) {
+	case "text", "string":
+		return onepassword.ItemFieldTypeText
+	case "concealed", "password":
+		return onepassword.ItemFieldTypeConcealed
+	case "email":
+		return onepassword.ItemFieldTypeEmail
+	case "url":
+		return onepassword.ItemFieldTypeURL
+	case "phone":
+		return onepassword.ItemFieldTypePhone
+	case "date":
+		return onepassword.ItemFieldTypeDate
+	case "monthyear":
+		return onepassword.ItemFieldTypeMonthYear
+	}
+	return onepassword.ItemFieldTypeConcealed
+}
+
+// normalizeItemFields clears empty section IDs because the 1Password SDK rejects items with a SectionID pointer to "" when the section is missing.
+func normalizeItemFields(fields []onepassword.ItemField) []onepassword.ItemField {
+	for i := range fields {
+		if fields[i].SectionID != nil && *fields[i].SectionID == "" {
+			fields[i].SectionID = nil
+		}
+	}
+	return fields
+}
+
+// generateNewItemField creates an ItemField with ID and Title set to the given title (unique within item), value, and field type.
+func generateNewItemField(title, newVal string, fieldType onepassword.ItemFieldType) onepassword.ItemField {
+	return onepassword.ItemField{
+		ID:        title,
 		Title:     title,
 		Value:     newVal,
-		FieldType: onepassword.ItemFieldTypeConcealed,
+		FieldType: fieldType,
 	}
-
-	return field
 }
 
 // PushSecret creates or updates a secret in 1Password.
 func (p *SecretsClient) PushSecret(ctx context.Context, secret *corev1.Secret, ref esv1.PushSecretData) error {
+	if ref.GetSecretKey() == "" {
+		return p.pushAllKeys(ctx, secret, ref)
+	}
+
 	val, ok := secret.Data[ref.GetSecretKey()]
 	if !ok {
 		return fmt.Errorf("secret %s/%s does not contain a key", secret.Namespace, secret.Name)
@@ -369,32 +422,32 @@ func (p *SecretsClient) PushSecret(ctx context.Context, secret *corev1.Secret, r
 	title := ref.GetRemoteKey()
 	providerItem, err := p.findItem(ctx, title)
 	if errors.Is(err, ErrKeyNotFound) {
-		if err = p.createItem(ctx, val, ref); err != nil {
-			return fmt.Errorf("failed to create item: %w", err)
-		}
-
-		return nil
+		return p.createItem(ctx, val, ref)
 	} else if err != nil {
 		return fmt.Errorf("failed to find item: %w", err)
 	}
 
-	// TODO: We are only sending info to a specific label on a 1password item.
-	// We should change this logic eventually to allow pushing whole kubernetes Secrets to 1password as multiple labels
-	// OOTB.
+	providerItem.Fields = normalizeItemFields(providerItem.Fields)
+
 	label := ref.GetProperty()
 	if label == "" {
-		label = "password"
+		label = defaultFieldLabel
 	}
 
 	mdata, err := metadata.ParseMetadataParameters[PushSecretMetadataSpec](ref.GetMetadata())
 	if err != nil {
-		return fmt.Errorf("failed to parse push secret metadata: %w", err)
+		return fmt.Errorf(errMsgParsePushMeta, err)
 	}
 	if mdata != nil && mdata.Spec.Tags != nil {
 		providerItem.Tags = mdata.Spec.Tags
 	}
 
-	providerItem.Fields, err = updateFieldValue(providerItem.Fields, label, string(val))
+	fieldType := onepassword.ItemFieldTypeConcealed
+	if mdata != nil {
+		fieldType = resolveFieldType(mdata.Spec.FieldType)
+	}
+
+	providerItem.Fields, err = updateFieldValue(providerItem.Fields, label, string(val), fieldType)
 	if err != nil {
 		return fmt.Errorf("failed to update field with label: %s: %w", label, err)
 	}
@@ -402,7 +455,7 @@ func (p *SecretsClient) PushSecret(ctx context.Context, secret *corev1.Secret, r
 	_, err = p.client.Items().Put(ctx, providerItem)
 	metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKItemsPut, err)
 	if err != nil {
-		return fmt.Errorf("failed to update item: %w", err)
+		return fmt.Errorf(errMsgUpdateItem, err)
 	}
 
 	p.invalidateCacheByPrefix(p.constructRefKey(title))
@@ -411,6 +464,83 @@ func (p *SecretsClient) PushSecret(ctx context.Context, secret *corev1.Secret, r
 	return nil
 }
 
+// createAllKeysItem creates a new item with all keys from secret.Data.
+func (p *SecretsClient) createAllKeysItem(ctx context.Context, secret *corev1.Secret, title string, tags []string, fieldType onepassword.ItemFieldType) error {
+	fields := make([]onepassword.ItemField, 0, len(secret.Data))
+	for k, v := range secret.Data {
+		fields = append(fields, generateNewItemField(k, string(v), fieldType))
+	}
+	_, err := p.client.Items().Create(ctx, onepassword.ItemCreateParams{
+		Category: onepassword.ItemCategoryServer,
+		VaultID:  p.vaultID,
+		Title:    title,
+		Fields:   fields,
+		Tags:     tags,
+	})
+	metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKItemsCreate, err)
+	if err != nil {
+		return fmt.Errorf(errMsgCreateItem, err)
+	}
+	p.invalidateCacheByPrefix(p.constructRefKey(title))
+	p.invalidateItemCache(title)
+	return nil
+}
+
+// pushAllKeys pushes all keys from secret.Data as separate fields on a single 1Password item.
+func (p *SecretsClient) pushAllKeys(ctx context.Context, secret *corev1.Secret, ref esv1.PushSecretData) error {
+	mdata, err := metadata.ParseMetadataParameters[PushSecretMetadataSpec](ref.GetMetadata())
+	if err != nil {
+		return fmt.Errorf(errMsgParsePushMeta, err)
+	}
+
+	var tags []string
+	if mdata != nil && mdata.Spec.Tags != nil {
+		tags = mdata.Spec.Tags
+	}
+
+	fieldType := onepassword.ItemFieldTypeConcealed
+	if mdata != nil {
+		fieldType = resolveFieldType(mdata.Spec.FieldType)
+	}
+
+	title := ref.GetRemoteKey()
+	providerItem, err := p.findItem(ctx, title)
+
+	if errors.Is(err, ErrKeyNotFound) {
+		return p.createAllKeysItem(ctx, secret, title, tags, fieldType)
+	}
+	if err != nil {
+		return fmt.Errorf("failed to find item: %w", err)
+	}
+
+	providerItem.Fields = normalizeItemFields(providerItem.Fields)
+	if tags != nil {
+		providerItem.Tags = tags
+	}
+	kept := make([]onepassword.ItemField, 0, len(providerItem.Fields))
+	for _, f := range providerItem.Fields {
+		if v, ok := secret.Data[f.Title]; ok {
+			f.Value = string(v)
+			f.FieldType = fieldType
+			kept = append(kept, f)
+		}
+	}
+	for k, v := range secret.Data {
+		if countFieldsWithLabel(k, kept) == 0 {
+			kept = append(kept, generateNewItemField(k, string(v), fieldType))
+		}
+	}
+	providerItem.Fields = kept
+	_, err = p.client.Items().Put(ctx, providerItem)
+	metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKItemsPut, err)
+	if err != nil {
+		return fmt.Errorf(errMsgUpdateItem, err)
+	}
+	p.invalidateCacheByPrefix(p.constructRefKey(title))
+	p.invalidateItemCache(title)
+	return nil
+}
+
 // GetVault retrieves a vault by its title or UUID from 1Password.
 func (p *SecretsClient) GetVault(ctx context.Context, titleOrUUID string) (string, error) {
 	vaults, err := p.client.VaultsAPI.List(ctx)
@@ -484,9 +614,27 @@ func (p *SecretsClient) findItem(ctx context.Context, name string) (onepassword.
 	return item, nil
 }
 
-// SecretExists checks if a secret exists in 1Password.
-func (p *SecretsClient) SecretExists(_ context.Context, _ esv1.PushSecretRemoteRef) (bool, error) {
-	return false, fmt.Errorf("not implemented")
+// SecretExists returns true if the item exists, and if a property is specified, if a field with that title exists.
+func (p *SecretsClient) SecretExists(ctx context.Context, ref esv1.PushSecretRemoteRef) (bool, error) {
+	item, err := p.findItem(ctx, ref.GetRemoteKey())
+	if errors.Is(err, ErrKeyNotFound) {
+		return false, nil
+	}
+	if err != nil {
+		return false, err
+	}
+
+	property := ref.GetProperty()
+	if property == "" {
+		return true, nil // item exists; pushAllKeys handles field-level reconciliation
+	}
+
+	for _, f := range item.Fields {
+		if f.Title == property {
+			return true, nil
+		}
+	}
+	return false, nil
 }
 
 // Validate does nothing here. It would be possible to ping the SDK to prove we're healthy, but

+ 346 - 6
providers/v1/onepasswordsdk/client_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	v1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
@@ -588,16 +589,23 @@ func TestGetVault(t *testing.T) {
 }
 
 type fakeLister struct {
-	listAllResult []onepassword.ItemOverview
-	createCalled  bool
-	putCalled     bool
-	deleteCalled  bool
-	getResult     onepassword.Item
-	fileLister    onepassword.ItemsFilesAPI
+	listAllResult    []onepassword.ItemOverview
+	createCalled     bool
+	createdFieldType onepassword.ItemFieldType
+	createdParams    onepassword.ItemCreateParams
+	putCalled        bool
+	putItem          onepassword.Item
+	deleteCalled     bool
+	getResult        onepassword.Item
+	fileLister       onepassword.ItemsFilesAPI
 }
 
 func (f *fakeLister) Create(ctx context.Context, params onepassword.ItemCreateParams) (onepassword.Item, error) {
 	f.createCalled = true
+	f.createdParams = params
+	if len(params.Fields) > 0 {
+		f.createdFieldType = params.Fields[0].FieldType
+	}
 	return onepassword.Item{}, nil
 }
 
@@ -607,6 +615,7 @@ func (f *fakeLister) Get(ctx context.Context, vaultID, itemID string) (onepasswo
 
 func (f *fakeLister) Put(ctx context.Context, item onepassword.Item) (onepassword.Item, error) {
 	f.putCalled = true
+	f.putItem = item
 	return onepassword.Item{}, nil
 }
 
@@ -1190,6 +1199,243 @@ var _ onepassword.ItemsAPI = &fakeLister{}
 var _ onepassword.SecretsAPI = &fakeClientWithCounter{}
 var _ onepassword.ItemsAPI = &fakeListerWithCounter{}
 
+func TestSecretExists(t *testing.T) {
+	fc := &fakeClient{
+		listAllResult: []onepassword.VaultOverview{
+			{ID: "vault-id", Title: "vault"},
+		},
+	}
+
+	itemWithPassword := &fakeLister{
+		listAllResult: []onepassword.ItemOverview{
+			{ID: "item-id", Title: "key", VaultID: "vault-id"},
+		},
+		getResult: onepassword.Item{
+			ID: "item-id", Title: "key", VaultID: "vault-id",
+			Fields: []onepassword.ItemField{
+				{Title: "password", Value: "s3cr3t"},
+			},
+		},
+	}
+
+	tests := []struct {
+		name        string
+		ref         v1alpha1.PushSecretRemoteRef
+		lister      *fakeLister
+		wantExists  bool
+		assertError func(t *testing.T, err error)
+	}{
+		{
+			name:        "item does not exist returns false",
+			ref:         v1alpha1.PushSecretRemoteRef{RemoteKey: "missing"},
+			lister:      &fakeLister{listAllResult: []onepassword.ItemOverview{}},
+			wantExists:  false,
+			assertError: func(t *testing.T, err error) { require.NoError(t, err) },
+		},
+		{
+			name:        "item exists no property returns true",
+			ref:         v1alpha1.PushSecretRemoteRef{RemoteKey: "key"},
+			lister:      itemWithPassword,
+			wantExists:  true,
+			assertError: func(t *testing.T, err error) { require.NoError(t, err) },
+		},
+		{
+			name:        "item exists field present returns true",
+			ref:         v1alpha1.PushSecretRemoteRef{RemoteKey: "key", Property: "password"},
+			lister:      itemWithPassword,
+			wantExists:  true,
+			assertError: func(t *testing.T, err error) { require.NoError(t, err) },
+		},
+		{
+			name:        "item exists field absent returns false",
+			ref:         v1alpha1.PushSecretRemoteRef{RemoteKey: "key", Property: "api-token"},
+			lister:      itemWithPassword,
+			wantExists:  false,
+			assertError: func(t *testing.T, err error) { require.NoError(t, err) },
+		},
+		{
+			name: "pushAllKeys scenario: item exists with no fields returns true",
+			ref:  v1alpha1.PushSecretRemoteRef{RemoteKey: "key"},
+			lister: &fakeLister{
+				listAllResult: []onepassword.ItemOverview{
+					{ID: "item-id", Title: "key", VaultID: "vault-id"},
+				},
+				getResult: onepassword.Item{
+					ID: "item-id", Title: "key", VaultID: "vault-id",
+					Fields: []onepassword.ItemField{},
+				},
+			},
+			wantExists:  true,
+			assertError: func(t *testing.T, err error) { require.NoError(t, err) },
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			p := &SecretsClient{
+				client: &onepassword.Client{
+					SecretsAPI: fc,
+					VaultsAPI:  fc,
+					ItemsAPI:   tt.lister,
+				},
+				vaultID: "vault-id",
+			}
+			exists, err := p.SecretExists(t.Context(), tt.ref)
+			tt.assertError(t, err)
+			assert.Equal(t, tt.wantExists, exists)
+		})
+	}
+}
+
+func TestResolveFieldType(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected onepassword.ItemFieldType
+	}{
+		{"text", onepassword.ItemFieldTypeText},
+		{"Text", onepassword.ItemFieldTypeText},
+		{"TEXT", onepassword.ItemFieldTypeText},
+		{"concealed", onepassword.ItemFieldTypeConcealed},
+		{"Concealed", onepassword.ItemFieldTypeConcealed},
+		{"url", onepassword.ItemFieldTypeURL},
+		{"URL", onepassword.ItemFieldTypeURL},
+		{"email", onepassword.ItemFieldTypeEmail},
+		{"Email", onepassword.ItemFieldTypeEmail},
+		{"phone", onepassword.ItemFieldTypePhone},
+		{"date", onepassword.ItemFieldTypeDate},
+		{"monthYear", onepassword.ItemFieldTypeMonthYear},
+		{"monthyear", onepassword.ItemFieldTypeMonthYear},
+		{"MONTHYEAR", onepassword.ItemFieldTypeMonthYear},
+		{"", onepassword.ItemFieldTypeConcealed},
+		{"unknown", onepassword.ItemFieldTypeConcealed},
+		{"otp", onepassword.ItemFieldTypeConcealed},
+		{"file", onepassword.ItemFieldTypeConcealed},
+	}
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			got := resolveFieldType(tt.input)
+			assert.Equal(t, tt.expected, got)
+		})
+	}
+}
+
+func TestPushSecretFieldType(t *testing.T) {
+	fc := &fakeClient{
+		listAllResult: []onepassword.VaultOverview{
+			{ID: "vault-id", Title: "vault"},
+		},
+	}
+
+	tests := []struct {
+		name          string
+		metadataJSON  string
+		wantFieldType onepassword.ItemFieldType
+	}{
+		{
+			name:          "no metadata defaults to Concealed",
+			metadataJSON:  "",
+			wantFieldType: onepassword.ItemFieldTypeConcealed,
+		},
+		{
+			name:          "fieldType text creates Text field",
+			metadataJSON:  `{"apiVersion":"kubernetes.external-secrets.io/v1alpha1","kind":"PushSecretMetadata","spec":{"fieldType":"text"}}`,
+			wantFieldType: onepassword.ItemFieldTypeText,
+		},
+		{
+			name:          "fieldType URL case-insensitive",
+			metadataJSON:  `{"apiVersion":"kubernetes.external-secrets.io/v1alpha1","kind":"PushSecretMetadata","spec":{"fieldType":"URL"}}`,
+			wantFieldType: onepassword.ItemFieldTypeURL,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			fl := &fakeLister{
+				listAllResult: []onepassword.ItemOverview{},
+			}
+			p := &SecretsClient{
+				client: &onepassword.Client{
+					SecretsAPI: fc,
+					VaultsAPI:  fc,
+					ItemsAPI:   fl,
+				},
+				vaultID: "vault-id",
+			}
+
+			ref := v1alpha1.PushSecretData{
+				Match: v1alpha1.PushSecretMatch{
+					SecretKey: "key",
+					RemoteRef: v1alpha1.PushSecretRemoteRef{RemoteKey: "item", Property: "field"},
+				},
+			}
+			if tt.metadataJSON != "" {
+				raw := apiextensionsv1.JSON{Raw: []byte(tt.metadataJSON)}
+				ref.Metadata = &raw
+			}
+
+			secret := &corev1.Secret{
+				Data: map[string][]byte{"key": []byte("value")},
+			}
+
+			err := p.PushSecret(t.Context(), secret, ref)
+			require.NoError(t, err)
+			require.True(t, fl.createCalled, "Create should have been called")
+			assert.Equal(t, tt.wantFieldType, fl.createdFieldType)
+		})
+	}
+}
+
+func TestUpdateFieldValueChangesFieldType(t *testing.T) {
+	// Regression test: updateFieldValue must update FieldType when spec.fieldType changes,
+	// not only when Value changes.
+	fields := []onepassword.ItemField{
+		{Title: "myfield", Value: "secret", FieldType: onepassword.ItemFieldTypeConcealed},
+	}
+
+	updated, err := updateFieldValue(fields, "myfield", "secret", onepassword.ItemFieldTypeText)
+	require.NoError(t, err)
+	require.Len(t, updated, 1)
+	assert.Equal(t, onepassword.ItemFieldTypeText, updated[0].FieldType, "FieldType should be updated even when Value is unchanged")
+	assert.Equal(t, "secret", updated[0].Value)
+}
+
+func TestGenerateNewItemFieldHasNonEmptyID(t *testing.T) {
+	// Regression test: fields created without an ID cause "duplicate field ids" errors
+	// when two PushSecret data entries target the same 1Password item.
+	// See: generateNewItemField must always produce a non-empty ID.
+	tests := []struct {
+		title     string
+		fieldType onepassword.ItemFieldType
+	}{
+		{"password", onepassword.ItemFieldTypeConcealed},
+		{"api-endpoint", onepassword.ItemFieldTypeURL},
+		{"username", onepassword.ItemFieldTypeText},
+	}
+	for _, tt := range tests {
+		field := generateNewItemField(tt.title, "value", tt.fieldType)
+		assert.NotEmpty(t, field.ID, "field ID must be non-empty to avoid duplicate ID errors on Put")
+		assert.Equal(t, tt.title, field.Title)
+		assert.Equal(t, "value", field.Value)
+	}
+}
+
+func TestNormalizeItemFields(t *testing.T) {
+	// Regression test: fields fetched from 1Password can have SectionID pointing to ""
+	// instead of nil. The SDK rejects Put when a field references a section ID that
+	// doesn't exist in item.Sections — even an empty-string pointer triggers this.
+	emptyStr := ""
+	realSection := "extra"
+	fields := []onepassword.ItemField{
+		{ID: "a", Title: "a", SectionID: &emptyStr},
+		{ID: "b", Title: "b", SectionID: nil},
+		{ID: "c", Title: "c", SectionID: &realSection},
+	}
+	got := normalizeItemFields(fields)
+	assert.Nil(t, got[0].SectionID, "empty-string SectionID should be normalized to nil")
+	assert.Nil(t, got[1].SectionID, "nil SectionID should remain nil")
+	assert.Equal(t, &realSection, got[2].SectionID, "non-empty SectionID should be unchanged")
+}
+
 func TestIsNativeItemID(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -1216,3 +1462,97 @@ func TestIsNativeItemID(t *testing.T) {
 		})
 	}
 }
+
+func TestPushAllKeys(t *testing.T) {
+	const (
+		testExistingItem = "existing-item"
+		testOldKey       = "old-key"
+	)
+
+	fc := &fakeClient{listAllResult: []onepassword.VaultOverview{{ID: "vault-id", Title: "vault"}}}
+
+	existingItem := onepassword.Item{
+		ID: "item-id", Title: testExistingItem, VaultID: "vault-id",
+		Fields: []onepassword.ItemField{
+			{ID: testOldKey, Title: testOldKey, Value: "old-val", FieldType: onepassword.ItemFieldTypeConcealed},
+		},
+	}
+
+	newLister := func(existing ...onepassword.Item) *fakeLister {
+		fl := &fakeLister{listAllResult: []onepassword.ItemOverview{}}
+		if len(existing) > 0 {
+			fl.getResult = existing[0]
+			fl.listAllResult = []onepassword.ItemOverview{{ID: existing[0].ID, Title: existing[0].Title, VaultID: existing[0].VaultID}}
+		}
+		return fl
+	}
+
+	fieldsMap := func(fields []onepassword.ItemField) map[string]onepassword.ItemField {
+		m := make(map[string]onepassword.ItemField, len(fields))
+		for _, f := range fields {
+			m[f.Title] = f
+		}
+		return m
+	}
+
+	ref := func(key, remoteKey string, meta ...string) v1alpha1.PushSecretData {
+		d := v1alpha1.PushSecretData{Match: v1alpha1.PushSecretMatch{SecretKey: key, RemoteRef: v1alpha1.PushSecretRemoteRef{RemoteKey: remoteKey}}}
+		if len(meta) > 0 {
+			raw := apiextensionsv1.JSON{Raw: []byte(meta[0])}
+			d.Metadata = &raw
+		}
+		return d
+	}
+
+	secret := func(kv ...string) *corev1.Secret {
+		s := &corev1.Secret{Data: map[string][]byte{}}
+		for i := 0; i+1 < len(kv); i += 2 {
+			s.Data[kv[i]] = []byte(kv[i+1])
+		}
+		return s
+	}
+
+	t.Run("creates new item with all secret keys as concealed fields", func(t *testing.T) {
+		fl := newLister()
+		p := &SecretsClient{client: &onepassword.Client{SecretsAPI: fc, VaultsAPI: fc, ItemsAPI: fl}, vaultID: "vault-id"}
+		require.NoError(t, p.PushSecret(t.Context(), secret("alpha", "val-alpha", "beta", "val-beta"), ref("", "my-item")))
+		require.True(t, fl.createCalled)
+		assert.False(t, fl.putCalled)
+		fm := fieldsMap(fl.createdParams.Fields)
+		assert.Equal(t, "val-alpha", fm["alpha"].Value)
+		assert.Equal(t, onepassword.ItemFieldTypeConcealed, fm["alpha"].FieldType)
+		assert.Equal(t, "val-beta", fm["beta"].Value)
+	})
+
+	t.Run("updates existing item with all secret keys", func(t *testing.T) {
+		fl := newLister(onepassword.Item{ID: "item-id", Title: testExistingItem, VaultID: "vault-id"})
+		p := &SecretsClient{client: &onepassword.Client{SecretsAPI: fc, VaultsAPI: fc, ItemsAPI: fl}, vaultID: "vault-id"}
+		require.NoError(t, p.PushSecret(t.Context(), secret("key1", "value1", "key2", "value2"), ref("", testExistingItem)))
+		assert.False(t, fl.createCalled)
+		require.True(t, fl.putCalled)
+		fm := fieldsMap(fl.putItem.Fields)
+		assert.Equal(t, "value1", fm["key1"].Value)
+		assert.Equal(t, "value2", fm["key2"].Value)
+	})
+
+	t.Run("applies tags from metadata on create", func(t *testing.T) {
+		fl := newLister()
+		p := &SecretsClient{client: &onepassword.Client{SecretsAPI: fc, VaultsAPI: fc, ItemsAPI: fl}, vaultID: "vault-id"}
+		meta := `{"apiVersion":"kubernetes.external-secrets.io/v1alpha1","kind":"PushSecretMetadata","spec":{"tags":["env:prod","team:backend"]}}`
+		require.NoError(t, p.PushSecret(t.Context(), secret("k", "v"), ref("", "tagged-item", meta)))
+		require.True(t, fl.createCalled)
+		assert.Equal(t, []string{"env:prod", "team:backend"}, fl.createdParams.Tags)
+	})
+
+	t.Run("removes fields deleted from the secret", func(t *testing.T) {
+		fl := newLister(existingItem) // existingItem has field testOldKey
+		p := &SecretsClient{client: &onepassword.Client{SecretsAPI: fc, VaultsAPI: fc, ItemsAPI: fl}, vaultID: "vault-id"}
+		// secret no longer contains testOldKey, only "new-key"
+		require.NoError(t, p.PushSecret(t.Context(), secret("new-key", "new-val"), ref("", testExistingItem)))
+		require.True(t, fl.putCalled)
+		fm := fieldsMap(fl.putItem.Fields)
+		assert.Equal(t, "new-val", fm["new-key"].Value, "new field must be added")
+		_, stillThere := fm[testOldKey]
+		assert.False(t, stillThere, "deleted key must be removed from the 1Password item")
+	})
+}

+ 1 - 1
providers/v1/onepasswordsdk/go.mod

@@ -9,6 +9,7 @@ require (
 	github.com/hashicorp/golang-lru/v2 v2.0.7
 	github.com/stretchr/testify v1.11.1
 	k8s.io/api v0.35.0
+	k8s.io/apiextensions-apiserver v0.35.0
 	k8s.io/apimachinery v0.35.0
 	sigs.k8s.io/controller-runtime v0.23.1
 )
@@ -91,7 +92,6 @@ require (
 	gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
-	k8s.io/apiextensions-apiserver v0.35.0 // indirect
 	k8s.io/client-go v0.35.0 // indirect
 	k8s.io/klog/v2 v2.130.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20260127142750-a19766b6e2d4 // indirect