deploy/crds/bundle.yaml (5)

3753-3798: Same v1-only schema verification applies to the Device42 block.
Please follow the earlier verification to ensure this block isn’t under v1beta1. Based on learnings, v1beta1 should not receive new providers.

---

5953-6027: Same v1-only schema verification applies to the Passbolt block.
Please follow the earlier verification to ensure this block isn’t under v1beta1. Based on learnings, v1beta1 should not receive new providers.

---

14229-14318: Same v1-only schema verification applies to this Alibaba block.
Please follow the earlier verification to ensure this block isn’t under v1beta1. Based on learnings, v1beta1 should not receive new providers.

---

15530-15575: Same v1-only schema verification applies to this Device42 block.
Please follow the earlier verification to ensure this block isn’t under v1beta1. Based on learnings, v1beta1 should not receive new providers.

---

17730-17804: Same v1-only schema verification applies to this Passbolt block.
Please follow the earlier verification to ensure this block isn’t under v1beta1. Based on learnings, v1beta1 should not receive new providers.

docs/snippets/passbolt-secret-example.yaml-5-7 (1)

5-7: Secret data field values must be base64-encoded.

Kubernetes Secret data fields require base64-encoded values. The current plaintext values will cause errors when applied. Either:

1. Use stringData instead of data for plaintext values, or
2. Base64-encode the values in data

Option 1: Use stringData (recommended for docs)

 apiVersion: v1
 kind: Secret
 metadata:
   name: passbolt-example
-data:
+stringData:
   full_secret: '{"name":"passbolt-secret","username":"some-username","password":"supersecretpassword","uri":"passbolt.com","description":"some description"}'
   password_only: supersecretpassword
 type: Opaque

apis/externalsecrets/v1/secretstore_device42_types.go-37-42 (1)

37-42: Consider aligning SecretRef optionality with other providers.

Device42Auth.SecretRef is a required field (non-pointer type), but Device42SecretRef.Credentials is optional. This deviates from the pattern used by other providers (AWS, GCPSM, Vault, Azure, etc.), which use pointers for SecretRef fields to make them optional. Either make SecretRef a pointer to align with other providers, or make Credentials required if authentication always needs credentials.

providers/v1/device42/device42.go-181-189 (1)

181-189: Missing nil client check.

GetSecretMap does not check if p.client is nil before calling GetSecret, unlike GetSecret which has this check on line 170. This inconsistency could lead to a nil pointer dereference.

🐛 Proposed fix

 func (p *Device42) GetSecretMap(_ context.Context, ref esv1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
+	if esutils.IsNil(p.client) {
+		return nil, errors.New(errUninitializedProvider)
+	}
 	data, err := p.client.GetSecret(ref.Key)
 	if err != nil {
 		return nil, fmt.Errorf("error getting secret %s: %w", ref.Key, err)
 	}
 
 	return data.ToMap(), nil
 }

pkg/register/device42.go-27-29 (1)

27-29: Incorrect comment: refers to "vault provider" instead of "device42 provider".

The comment on line 28 appears to be a copy-paste error.

Suggested fix

 func init() {
-	// Register vault provider
+	// Register device42 provider
 	esv1.Register(device42.NewProvider(), device42.ProviderSpec(), device42.MaintenanceStatus())
 }

docs/provider/device42.md-6-6 (1)

6-6: Fix grammar in auth and secret reference descriptions.
Keeps docs professional and unambiguous.

✏️ Suggested edits

-`username` and `password` is required to talk to the Device42 API.
+`username` and `password` are required to talk to the Device42 API.
...
-The `password` field is return from device42
+The `password` field is returned from Device42.

Also applies to: 39-40

docs/snippets/passbolt-external-secret-example.yaml-15-18 (1)

15-18: Fix typo in comments (“exising” → “existing”).
Small but user-facing polish.

🔧 Suggested edit

-      key: e22487a8-feb8-4591-95aa-14b193930cb4 # Replace with ID of exising Passbolt secret
+      key: e22487a8-feb8-4591-95aa-14b193930cb4 # Replace with ID of existing Passbolt secret
...
-      key: e22487a8-feb8-4591-95aa-14b193930cb4 # Replace with ID of exising Passbolt secret
+      key: e22487a8-feb8-4591-95aa-14b193930cb4 # Replace with ID of existing Passbolt secret

providers/v1/alibaba/kms.go-310-312 (1)

310-312: Typo in error message: "proivder" should be "provider".

✏️ Proposed fix

 	if alibabaSpec.Auth.RRSAAuth.OIDCProviderARN == "" {
-		return errors.New("missing alibaba OIDC proivder ARN")
+		return errors.New("missing alibaba OIDC provider ARN")
 	}

providers/v1/passbolt/passbolt.go-192-195 (1)

192-195: Potential nil pointer dereference if Close is called before NewClient.

If Close is called when provider.client is nil (e.g., if NewClient was never called or failed), this will cause a panic.

🛡️ Proposed defensive fix

 // Close implements cleanup operations for the Passbolt provider.
 func (provider *ProviderPassbolt) Close(ctx context.Context) error {
+	if provider.client == nil {
+		return nil
+	}
 	return provider.client.Logout(ctx)
 }

providers/v1/passbolt/passbolt.go-295-298 (1)

295-298: Empty totp case and incorrect error formatting.

Two issues in this switch statement:

1. The totp case (line 295-296) is empty and doesn't populate any password-related fields. If this is intentional behavior for TOTP-only resources, consider adding a comment to clarify.

2. Line 297 uses resourceType (the struct) in the format string with %q, which will print the struct representation rather than the slug.

🐛 Proposed fix for error formatting

 	case "totp":
+		// TOTP resources don't have a password field
 	default:
-		return nil, fmt.Errorf("UnknownPassboltResourceType: %q", resourceType)
+		return nil, fmt.Errorf("UnknownPassboltResourceType: %q", resourceType.Slug)
 	}

providers/v1/passbolt/passbolt_test.go (1)

268-280: Minor: Subtest receives unused *testing.T parameter.

The subtest function signature discards *testing.T while relying on the outer t registered with gomega. This works but is inconsistent with the pattern used in TestGetAllSecrets (lines 199-225) which properly uses the inner t.

♻️ Suggested fix

 	for _, row := range tbl {
-		t.Run(row.name, func(_ *testing.T) {
+		t.Run(row.name, func(t *testing.T) {
+			g.RegisterTestingT(t)
 			p := &ProviderPassbolt{client: clientMock}

apis/externalsecrets/v1/secretstore_device42_types.go (1)

24-30: Doc comment inconsistency: "URL" vs "Host".

The comment on line 25 states "URL configures the Device42 instance URL" but the field is named Host. Consider aligning the comment with the field name for clarity.

📝 Suggested fix

 // Device42Provider configures a store to sync secrets with a Device42 instance.
 type Device42Provider struct {
-	// URL configures the Device42 instance URL.
+	// Host configures the Device42 instance URL.
 	Host string `json:"host"`

providers/v1/alibaba/fake/fake.go (1)

31-34: Mock ignores the actual request parameter.

GetSecretValue creates a new empty GetSecretValueRequest instead of forwarding the actual request to the mock function. This prevents tests from verifying that the correct request parameters are being passed.

♻️ Suggested fix

 // GetSecretValue retrieves a secret value from the mock Alibaba client.
-func (mc *AlibabaMockClient) GetSecretValue(context.Context, *kmssdk.GetSecretValueRequest) (result *kmssdk.GetSecretValueResponseBody, err error) {
-	return mc.getSecretValue(&kmssdk.GetSecretValueRequest{})
+func (mc *AlibabaMockClient) GetSecretValue(_ context.Context, req *kmssdk.GetSecretValueRequest) (result *kmssdk.GetSecretValueResponseBody, err error) {
+	return mc.getSecretValue(req)
 }

providers/v1/device42/device42_api.go (1)

64-78: Consider adding a default HTTP client timeout.

The HTTP client is created without a timeout. While GetSecret uses a context timeout, having a default client timeout provides defense-in-depth against hanging connections.

♻️ Suggested improvement

 func NewAPI(baseURL, username, password, hostPort string) *API {
 	tr := &http.Transport{
 		TLSClientConfig: &tls.Config{MinVersion: tls.VersionTLS12},
 	}
 	api := &API{
 		baseURL:  baseURL,
 		hostPort: hostPort,
 		username: username,
 		password: password,
-		client:   &http.Client{Transport: tr},
+		client:   &http.Client{Transport: tr, Timeout: 30 * time.Second},
 	}
 
 	return api
 }

providers/v1/device42/device42_api_test.go (1)

41-103: Consider adding test cases for edge scenarios.

The tests cover the happy path and bad JSON response, but don't cover:

1. Empty passwords array (which currently has a bug returning nil error)
2. Non-2xx HTTP status codes
3. Network/request errors

These additional test cases would help catch the issues identified in the implementation.

♻️ Additional test cases to consider

{
	name: "empty passwords array",
	fields: fields{
		funcStack: []func(req *http.Request) (*http.Response, error){
			createResponder(D42PasswordResponse{Passwords: []D42Password{}}, true),
		},
	},
	args: args{
		secretID: device42PasswordID,
	},
	want:    D42Password{},
	wantErr: true, // Should error when no password found
},
{
	name: "non-2xx status code",
	fields: fields{
		funcStack: []func(req *http.Request) (*http.Response, error){
			func(req *http.Request) (*http.Response, error) {
				return &http.Response{
					StatusCode: http.StatusUnauthorized,
					Body:       &closeableBuffer{bytes.NewReader([]byte("unauthorized"))},
				}, nil
			},
		},
	},
	args: args{
		secretID: device42PasswordID,
	},
	want:    D42Password{},
	wantErr: true,
},

providers/v1/device42/device42.go (2)

131-132: Port "443" is hardcoded.

The port is hardcoded to "443" but the Device42Provider struct likely has a configurable host field. Consider making the port configurable or documenting that only HTTPS on port 443 is supported.

---

54-57: ValidateStore performs no validation.

This method returns nil, nil without validating the store configuration. Consider adding basic validation (e.g., required fields like Host, Auth.SecretRef.Credentials.Name) to catch configuration errors early.

providers/v1/alibaba/logger.go (1)

29-33: Redundant iota declarations.

Each constant uses iota separately, which works but is unconventional. A single iota sequence is cleaner.

Suggested simplification

 const (
-	logLevelWarn  logLevel = iota
-	logLevelInfo  logLevel = iota
-	logLevelDebug logLevel = iota
+	logLevelWarn logLevel = iota
+	logLevelInfo
+	logLevelDebug
 )

apis/externalsecrets/v1/secretstore_types.go (1)

196-197: Missing descriptive comment for Passbolt provider.

Other provider fields include a comment describing the provider's purpose (e.g., "Alibaba configures this store to sync secrets using Alibaba Cloud provider"). The Passbolt field lacks this documentation.

Suggested fix

+	// Passbolt configures this store to sync secrets using the Passbolt provider
 	// +optional
 	Passbolt *PassboltProvider `json:"passbolt,omitempty"`

providers/v1/alibaba/kms_test.go (2)

127-132: Misleading variable name: successCases includes error cases.

The slice contains both success and error scenarios (e.g., setAPIErr, setNilMockClient), making successCases a misnomer. Consider renaming to testCases for clarity.

Suggested fix

-	successCases := []*keyManagementServiceTestCase{
+	testCases := []*keyManagementServiceTestCase{
 		makeValidKMSTestCaseCustom(setSecretString),
 		makeValidKMSTestCaseCustom(setCustomKey),
 		makeValidKMSTestCaseCustom(setAPIErr),
 		makeValidKMSTestCaseCustom(setNilMockClient),
 	}

Apply the same change at line 161.

---

181-237: Consider adding negative validation test cases.

The validation tests only cover valid configurations. Adding tests for invalid stores (e.g., missing RegionID, missing both auth methods, or providing both auth methods) would improve coverage.

apis/externalsecrets/v1/secretsstore_passbolt_types.go (1)

1-36: Filename inconsistency: extra 's' in filename.

The filename is secretsstore_passbolt_types.go but other provider type files follow the pattern secretstore_*_types.go (e.g., secretstore_alibaba_types.go, secretstore_device42_types.go). Consider renaming for consistency.

Rename file to: secretstore_passbolt_types.go

providers/v1/passbolt/passbolt.go (1)

141-144: Consider implementing actual validation for better UX.

The Validate() method always returns ValidationResultUnknown. While this is safe, it means users won't get feedback about whether their Passbolt configuration is correct until they try to fetch a secret. Consider validating by calling CheckSession or performing a test login.