providers/v1/nebius/common/sdk/sdk.go (1)

37-43: Simplify the nil check.

The caCertificate != nil check is redundant since len(nil) == 0 in Go for slices.

♻️ Suggested simplification

-	if caCertificate != nil && len(caCertificate) > 0 {
+	if len(caCertificate) > 0 {
		certPool := x509.NewCertPool()
		if ok := certPool.AppendCertsFromPEM(caCertificate); !ok {
			return nil, errors.New("failed to append CA certificate. PEM parse error")
		}
		tlsCfg.RootCAs = certPool
	}

providers/v1/nebius/common/sdk/mysterybox/fake/fake_client.go (3)

38-47: Inconsistent receiver types between methods.

Close() at line 33 uses a pointer receiver *FakeMysteryboxClient, while GetSecret and GetSecretByKey use value receivers. While this works functionally, it's inconsistent style. Consider using pointer receivers consistently for all methods.

♻️ Suggested fix

-func (f FakeMysteryboxClient) GetSecret(_ context.Context, _, secretId, versionId string) (*mysterybox.Payload, error) {
+func (f *FakeMysteryboxClient) GetSecret(_ context.Context, _, secretId, versionId string) (*mysterybox.Payload, error) {

---

49-64: Apply same receiver fix here for consistency.

♻️ Suggested fix

-func (f FakeMysteryboxClient) GetSecretByKey(_ context.Context, _, secretID, versionID, key string) (*mysterybox.PayloadEntry, error) {
+func (f *FakeMysteryboxClient) GetSecretByKey(_ context.Context, _, secretID, versionID, key string) (*mysterybox.PayloadEntry, error) {

---

66-74: Consider adding mutex for thread safety in parallel tests.

MysteryboxService accesses secretData map without synchronization. If tests using this fake service run in parallel and share the same service instance, this could cause data races.

♻️ Suggested fix

+import "sync"
+
 type MysteryboxService struct {
+	mu         sync.RWMutex
 	secretData map[string]map[string][]mysterybox.Entry
 }

Then add lock/unlock calls in GetSecret, CreateSecret, and CreateNewSecretVersion methods.

providers/v1/nebius/mysterybox/token_service_test.go (2)

119-121: Confusing parameter type in addSecondsToClock helper.

The parameter second has type time.Duration but is multiplied by time.Second, which is confusing. If a caller passes 5 * time.Second, the actual advancement would be 5 seconds × 1 second = 5,000,000,000 nanoseconds² (incorrect). The current usage passes integer literals which work due to implicit conversion, but this is fragile.

♻️ Proposed fix - use int64 for clarity

-func addSecondsToClock(clk *clocktesting.FakeClock, second time.Duration) {
-	clk.SetTime(clk.Now().Add(second * time.Second))
+func addSecondsToClock(clk *clocktesting.FakeClock, seconds int) {
+	clk.SetTime(clk.Now().Add(time.Duration(seconds) * time.Second))
 }

---

200-203: Consider checking errors in key rotation test.

The errors returned by GetToken are discarded. While the test focuses on cache key behavior, ignoring errors could hide unexpected failures during test execution.

♻️ Proposed fix

-	t1, _ := env.TokenCacheService.GetToken(ctx, "api", base, nil)
-	t2, _ := env.TokenCacheService.GetToken(ctx, "api", rotatedKeyID, nil)
-	t3, _ := env.TokenCacheService.GetToken(ctx, "api", rotatedPriv, nil)
-	t4, _ := env.TokenCacheService.GetToken(ctx, "api", rotatedSubject, nil)
+	t1, err := env.TokenCacheService.GetToken(ctx, "api", base, nil)
+	tassert.NoError(t, err)
+	t2, err := env.TokenCacheService.GetToken(ctx, "api", rotatedKeyID, nil)
+	tassert.NoError(t, err)
+	t3, err := env.TokenCacheService.GetToken(ctx, "api", rotatedPriv, nil)
+	tassert.NoError(t, err)
+	t4, err := env.TokenCacheService.GetToken(ctx, "api", rotatedSubject, nil)
+	tassert.NoError(t, err)

providers/v1/nebius/common/sdk/mysterybox/grpc_client.go (1)

130-134: Consider using named fields for struct initialization.

Using positional struct initialization can be fragile if the struct fields are reordered in the future. Named fields improve readability and maintainability.

♻️ Suggested refactor

 	return &GrpcClient{
-		mbox.NewPayloadService(mysteryboxSdk),
-		mysteryboxSdk,
+		PayloadService: mbox.NewPayloadService(mysteryboxSdk),
+		sdk:            mysteryboxSdk,
 	}, nil

providers/v1/nebius/mysterybox/validation_test.go (1)

194-206: Minor: Redundant nil check after assertion.

The if err != nil check on line 202 is redundant since tassert.NotNil on line 201 would have already failed the test if err was nil. Consider simplifying.

♻️ Suggested simplification

 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			_, err := p.ValidateStore(tt.store)
 			if tt.wantErr == "" {
 				tassert.NoError(t, err, "%s: unexpected error", tt.name)
 				return
 			}
-			tassert.NotNil(t, err, "%s: expected error containing %q, got nil", tt.name, tt.wantErr)
-			if err != nil {
-				tassert.Contains(t, err.Error(), tt.wantErr, "%s: error %q does not contain %q", tt.name, err.Error(), tt.wantErr)
-			}
+			tassert.Error(t, err, "%s: expected error containing %q, got nil", tt.name, tt.wantErr)
+			tassert.Contains(t, err.Error(), tt.wantErr, "%s: error %q does not contain %q", tt.name, err.Error(), tt.wantErr)
 		})
 	}

providers/v1/nebius/common/sdk/iam/grpc_client.go (2)

51-58: Consider including original error details in the returned error.

When JSON unmarshalling fails, the original error is only passed to the observer but not included in the returned error. This loses diagnostic information that could help users troubleshoot malformed credentials.

♻️ Suggested improvement

 	parsedSubjectCreds := &auth.ServiceAccountCredentials{}
 	if err := json.Unmarshal([]byte(subjectCreds), parsedSubjectCreds); err != nil {
 		if t.exchangeTokenObserveCall != nil {
 			t.exchangeTokenObserveCall(err)
 		}
-		return nil, errors.New(errInvalidSubjectCreds)
+		return nil, fmt.Errorf("%s: %w", errInvalidSubjectCreds, err)
 	}

---

86-92: Clarify metrics observation on success path.

The observer is called with err (which may be nil on success) before checking if err != nil. This appears intentional for metrics tracking, but a brief comment would clarify the design choice.

📝 Optional: Add clarifying comment

 	tok, err := tokenExchanger.Exchange(ctx, req)
+	// Always observe the result for metrics, regardless of success/failure
 	if t.exchangeTokenObserveCall != nil {
 		t.exchangeTokenObserveCall(err)
 	}

providers/v1/nebius/mysterybox/provider_test.go (1)

761-779: Non-atomic increment in fakeTokenService.

f.calls++ is not atomic, but since this fake is used in single-threaded test contexts, it's acceptable. If concurrent usage is anticipated, consider using atomic.AddInt32.

providers/v1/nebius/mysterybox/token_service.go (1)

88-95: Unsafe type assertion could panic if cache contains unexpected type.

The type assertion value.(*iam.Token) will panic if the cache somehow contains a value of a different type. Use the comma-ok idiom for defensive coding.

♻️ Suggested fix

 	value, ok := c.tokenCache.Get(*cacheKey)
 	if ok {
-		token := value.(*iam.Token)
-		tokenExpired := isTokenExpired(token, c.Clock)
-		if !tokenExpired {
-			return token.Token, nil
+		if token, ok := value.(*iam.Token); ok {
+			if !isTokenExpired(token, c.Clock) {
+				return token.Token, nil
+			}
 		}
 	}

providers/v1/nebius/mysterybox/provider.go (1)

35-35: Consider a more descriptive import alias.

The alias mysterybox2 doesn't convey its purpose. Consider mysteryboxsdk or similar to clarify it's the SDK client package.

♻️ Suggested improvement

-	mysterybox2 "github.com/external-secrets/external-secrets/providers/v1/nebius/common/sdk/mysterybox"
+	mysteryboxsdk "github.com/external-secrets/external-secrets/providers/v1/nebius/common/sdk/mysterybox"

docs/introduction/stability-support.md (1)

143-143: Missing entry in Provider Stability table.

The Nebius Mysterybox provider is added to the Feature Support table but still lacks a corresponding entry in the Provider Stability and Support Level table (lines 65-100). Please add an entry declaring stability level and maintainer.

docs/provider/nebius-mysterybox.md (1)

44-51: Wrap the include directives in YAML code fences.

Same issue as previously noted: without fences these includes render as plain text.

providers/v1/nebius/mysterybox/provider_test.go (2)

638-646: Using t.Errorf inside a goroutine may not properly report test failures.

When t.Errorf is called from a goroutine, the test may exit before the error is reported, potentially causing a panic. Use a channel or atomic.Value to collect errors and check them after wg.Wait().

---

727-759: Global state mutation without cleanup may affect parallel tests.

Setting mysteryboxClientsCacheSize = 1 modifies package-level state without restoring it. Consider using t.Cleanup to restore the original value, or restructure the test to inject the cache size.

config/crds/bases/external-secrets.io_clustersecretstores.yaml (1)

3716-3787: Require exactly one credential in auth.
auth is required but both credential refs are optional, so auth: {} validates and will fail at runtime. Add a CEL rule to enforce exactly one of serviceAccountCredsSecretRef or tokenSecretRef.

🔧 Proposed CRD validation

                   auth:
                     description: Auth defines parameters to authenticate in Mysterybox
                     properties:
                       serviceAccountCredsSecretRef:
@@
                       tokenSecretRef:
@@
-                    type: object
+                    type: object
+                    x-kubernetes-validations:
+                    - message: Exactly one of 'serviceAccountCredsSecretRef' or 'tokenSecretRef' must be specified
+                      rule: (has(self.serviceAccountCredsSecretRef) && !has(self.tokenSecretRef)) || (!has(self.serviceAccountCredsSecretRef) && has(self.tokenSecretRef))

apis/externalsecrets/v1/secretstore_nebius_types.go (2)

19-38: Require at least one auth source in NebiusAuth.
Right now auth: {} passes validation but fails at runtime. Add a CRD rule to require either serviceAccountCredsSecretRef or tokenSecretRef.

Suggested fix

 // NebiusAuth defines the authentication method for the Nebius provider.
+// +kubebuilder:validation:XValidation:rule="has(self.serviceAccountCredsSecretRef) || has(self.tokenSecretRef)",message="either serviceAccountCredsSecretRef or tokenSecretRef must be set"
 type NebiusAuth struct {

---

55-57: Avoid serializing caProvider: null.
CAProvider is optional but missing omitempty, so nil pointers serialize as null. Align with other optional provider fields.

Suggested fix

-	CAProvider *NebiusCAProvider `json:"caProvider"`
+	CAProvider *NebiusCAProvider `json:"caProvider,omitempty"`

config/crds/bases/external-secrets.io_secretstores.yaml (1)

3709-3824: Require at least one Nebius auth method (repeat).

auth is required but both serviceAccountCredsSecretRef and tokenSecretRef are optional, so auth: {} passes validation and will likely fail at runtime. Please add a schema rule (oneOf/anyOf or x-kubernetes-validations) to require at least one.

providers/v1/nebius/mysterybox/secrets_client.go (1)

63-66: Error message is misleading - operation is marshalling, not unmarshalling.

The error constant errJSONUnmarshal and message "failed to unmarshal JSON" are incorrect here since json.Marshal is being called.

🐛 Suggested fix

Update the error constant and usage:

 const (
 	errNotImplemented             = "not implemented"
-	errJSONUnmarshal              = "failed to unmarshal JSON"
+	errJSONMarshal                = "failed to marshal JSON"
 	...
 )

 		out, err := json.Marshal(keyToValue)
 		if err != nil {
-			return nil, errors.New(errJSONUnmarshal)
+			return nil, fmt.Errorf("%s: %w", errJSONMarshal, err)
 		}

providers/v1/nebius/common/sdk/mysterybox/client.go (1)

1-13: Unusual license header format.

The license header uses // /* which appears to be double-commented. This differs from the standard license format used elsewhere in the codebase. Consider using the standard format without the leading // on each line.

♻️ Suggested fix

-// /*
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-// */
+/*
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/

providers/v1/nebius/common/sdk/mysterybox/grpc_client_test.go (2)

1-13: Unusual license header format.

Same issue as in client.go - the license header uses double-comment format (// /*). Consider using the standard block comment format for consistency.

---

159-172: Simplify payload entry validation.

The validation logic copies tt.expected into a new map, then deletes matched entries - but the copy on lines 159-162 is unnecessary since tt.expected is already a map[string][]byte. Also, the assertion argument order should be (expected, actual) per testify conventions.

♻️ Suggested simplification

-			expected := make(map[string][]byte, len(tt.expected))
-			for k, v := range tt.expected {
-				expected[k] = v
-			}
-
-			tassert.Equal(t, len(payload.Entries), len(expected))
-			for _, entry := range payload.Entries {
-				value, _ := expected[entry.Key]
-				if (entry.BinaryValue != nil && bytes.Equal(value, entry.BinaryValue)) || (entry.StringValue != "" && bytes.Equal(value, []byte(entry.StringValue))) {
-					delete(expected, entry.Key)
-					continue
-				}
-			}
-			tassert.Empty(t, expected, "not all expected entries found: %+v", expected)
+			tassert.Equal(t, len(tt.expected), len(payload.Entries))
+			for _, entry := range payload.Entries {
+				expectedValue, ok := tt.expected[entry.Key]
+				tassert.True(t, ok, "unexpected key: %s", entry.Key)
+				if entry.BinaryValue != nil {
+					tassert.Equal(t, expectedValue, entry.BinaryValue)
+				} else {
+					tassert.Equal(t, expectedValue, []byte(entry.StringValue))
+				}
+			}

providers/v1/nebius/common/sdk/mysterybox/fake/fake_client.go (2)

1-13: Unusual license header format.

Same issue as other files - consider using standard block comment format.

---

38-47: Inconsistent receiver types and potential version ID issue.

Two concerns:

1. GetSecret and GetSecretByKey use value receivers while Close uses a pointer receiver. For consistency and to avoid copying the struct, consider using pointer receivers throughout.

2. In GetSecretByKey (line 57), when versionID is empty (requesting latest), the returned PayloadEntry.VersionID will be empty instead of the actual version ID from the secret. This differs from GetSecret which correctly returns secret.VersionId.

♻️ Suggested fix

-func (f FakeMysteryboxClient) GetSecret(_ context.Context, _, secretId, versionId string) (*mysterybox.Payload, error) {
+func (f *FakeMysteryboxClient) GetSecret(_ context.Context, _, secretId, versionId string) (*mysterybox.Payload, error) {

-func (f FakeMysteryboxClient) GetSecretByKey(_ context.Context, _, secretID, versionID, key string) (*mysterybox.PayloadEntry, error) {
+func (f *FakeMysteryboxClient) GetSecretByKey(_ context.Context, _, secretID, versionID, key string) (*mysterybox.PayloadEntry, error) {
 	secret, err := f.mysteryboxService.GetSecret(secretID, versionID)
 	if err != nil {
 		return nil, err
 	}
 	for _, entry := range secret.Entries {
 		if entry.Key == key {
 			return &mysterybox.PayloadEntry{
-				VersionID: versionID,
+				VersionID: secret.VersionId,
 				Entry:     entry,
 			}, nil
 		}
 	}

Also applies to: 49-64

providers/v1/nebius/mysterybox/validation_test.go (1)

290-351: LGTM with minor suggestion: TestValidateStore_APIDomainCases provides thorough domain validation coverage.

The test comprehensively validates:

• Valid domain formats (simple, subdomain, with port, hyphen, uppercase)
• Invalid formats (single label, empty label, leading/trailing dots, hyphen positioning, invalid characters)
• Length constraints (label > 63 chars, overall domain length)

Minor suggestion: The error containment check at lines 346-348 could be made conditional to improve clarity:

 if tc.valid {
     tassert.NoError(t, err, "%s: expected valid, got error", tc.name)
 } else {
     tassert.Error(t, err, "%s: expected error for domain %q", tc.name, tc.domain)
-}
-if err != nil {
     tassert.Contains(t, err.Error(), errInvalidAPIDomain, "%s: error should contain invalid api domain", tc.name)
 }

This makes it explicit that the error message check only applies to invalid cases.

providers/v1/nebius/mysterybox/provider.go (1)

35-35: Consider using a clearer import alias.

The alias mysterybox2 is unconventional. Consider a more descriptive alias like mysteryboxsdk or mbclient to clarify it refers to the SDK client package.

♻️ Suggested change

-	mysterybox2 "github.com/external-secrets/external-secrets/providers/v1/nebius/common/sdk/mysterybox"
+	mysteryboxsdk "github.com/external-secrets/external-secrets/providers/v1/nebius/common/sdk/mysterybox"

providers/v1/nebius/mysterybox/token_service.go (1)

81-105: Coalesce concurrent cache misses to avoid duplicate exchanges.
Parallel callers can miss the cache and exchange in parallel; consider a per‑key mutex to coalesce those calls and reduce redundant IAM requests.

providers/v1/nebius/common/sdk/mysterybox/grpc_client.go (1)

65-79: Potential nil dereference when entry.GetData() returns nil.

This issue was previously flagged. If the gRPC response's Data field is nil, accessing entry.GetData().GetKey() or entry.GetData().Payload will cause a panic.

providers/v1/nebius/mysterybox/secrets_client.go (1)

65-68: Error message is misleading - operation is marshalling, not unmarshalling.

This issue was previously flagged. The error constant errJSONUnmarshal and message "failed to unmarshal JSON" are incorrect since json.Marshal is being called.

providers/v1/nebius/mysterybox/provider.go (1)

230-248: Error from sync.Once is not properly propagated on subsequent calls.

This issue was previously flagged. If lru.NewWithEvict fails, the error is captured but cacheOnce.Do will never execute again on subsequent calls. This means later calls to initMysteryboxClientsCache() will return nil error even though initialization failed.

providers/v1/nebius/common/sdk/mysterybox/grpc_client.go (2)

101-117: Same nil dereference risk in GetSecret method.

The GetSecret method iterates over payload.GetData() entries and accesses entry.Key and entry.Payload directly without checking if entries could be nil. While GetData() returns a slice (less likely to be nil itself), individual entries within could still have nil fields depending on the protobuf generation.

More critically, this duplicates the payload-type switch logic from GetSecretByKey. Consider extracting a helper function to reduce duplication and ensure consistent handling.

♻️ Suggested refactor to extract common payload conversion

func convertPayloadToEntry(data *proto.Payload, key string) (Entry, error) {
	if data == nil {
		return Entry{}, fmt.Errorf("received nil data for key: %v", key)
	}
	entry := Entry{Key: data.GetKey()}
	switch data.Payload.(type) {
	case *proto.Payload_StringValue:
		entry.StringValue = data.GetStringValue()
	case *proto.Payload_BinaryValue:
		entry.BinaryValue = data.GetBinaryValue()
	default:
		return Entry{}, fmt.Errorf(notSupportedPayloadType, key)
	}
	return entry, nil
}

Then use this helper in both GetSecretByKey and GetSecret.

---

132-135: Use named fields in struct literal for clarity.

The struct literal uses positional initialization which can lead to subtle bugs if field order changes.

♻️ Suggested fix

 	return &GrpcClient{
-		mbox.NewPayloadService(mysteryboxSdk),
-		mysteryboxSdk,
+		PayloadService: mbox.NewPayloadService(mysteryboxSdk),
+		sdk:            mysteryboxSdk,
 	}, nil

providers/v1/nebius/common/sdk/mysterybox/fake/fake_client.go (1)

35-49: Inconsistent receiver types between methods.

Close() uses a pointer receiver *FakeMysteryboxClient, while GetSecret() and GetSecretByKey() use value receivers FakeMysteryboxClient. This is inconsistent and could cause issues since value receivers won't see mutations to the Closed field.

♻️ Suggested fix

-func (f FakeMysteryboxClient) GetSecret(_ context.Context, _, secretId, versionId string) (*mysterybox.Payload, error) {
+func (f *FakeMysteryboxClient) GetSecret(_ context.Context, _, secretId, versionId string) (*mysterybox.Payload, error) {

-func (f FakeMysteryboxClient) GetSecretByKey(_ context.Context, _, secretID, versionID, key string) (*mysterybox.PayloadEntry, error) {
+func (f *FakeMysteryboxClient) GetSecretByKey(_ context.Context, _, secretID, versionID, key string) (*mysterybox.PayloadEntry, error) {

providers/v1/nebius/mysterybox/secrets_client.go (3)

119-122: Close() does not close the underlying mysteryboxClient.

The Close method returns nil without closing the mysteryboxClient. Since the client is cached at the provider level, this may be intentional, but it's inconsistent with the mysterybox.Client interface which has a Close() method. Consider adding a comment explaining why the client isn't closed here (i.e., it's managed by the provider's cache).

📝 Suggested clarification

 // Close cleans up resources when the provider is done being used.
+// Note: The underlying mysteryboxClient is not closed here because it is
+// managed by the Provider's client cache and may be shared across multiple SecretsClient instances.
 func (c *SecretsClient) Close(_ context.Context) error {
 	return nil
 }

---

109-112: Validate() always returns Ready without performing validation.

The Validate method unconditionally returns ValidationResultReady without checking if the client can actually connect to the Mysterybox service. This could mask configuration issues. Consider adding at least a basic connectivity check or documenting why validation is skipped.

---

163-171: Unreachable code path in handleGetSecretByKeyError.

The condition if ref.Property != "" on line 164 will always be true when this function is called, because GetSecretByKey is only invoked when secretKey := ref.Property is non-empty (see line 54). The outer if ref.Property != "" check is redundant.

♻️ Simplified version

 func handleGetSecretByKeyError(err error, ref esv1.ExternalSecretDataRemoteRef) error {
 	if err == nil {
 		return nil
 	}
 	st, ok := status.FromError(err)
 	if !ok {
 		return err // not a grpc error
 	}
 	if st.Code() == codes.NotFound {
-		if ref.Property != "" {
-			if ref.Version != "" {
-				return fmt.Errorf(errSecretVersionByKeyNotFound, ref.Version, ref.Key, ref.Property, esv1.NoSecretErr)
-			}
-			return fmt.Errorf(errSecretByKeyNotFound, ref.Property, ref.Key, esv1.NoSecretErr)
+		if ref.Version != "" {
+			return fmt.Errorf(errSecretVersionByKeyNotFound, ref.Version, ref.Key, ref.Property, esv1.NoSecretErr)
 		}
+		return fmt.Errorf(errSecretByKeyNotFound, ref.Property, ref.Key, esv1.NoSecretErr)
 	}
 	return MapGrpcErrors("get secret", err)
 }

providers/v1/nebius/mysterybox/provider.go (1)

236-242: Variable shadowing: err in eviction callback shadows outer err.

The err variable declared on line 231 is shadowed by err on line 238 inside the eviction callback. If Close() fails, this inner err is logged but doesn't affect the outer err that is returned. While the current logic handles this correctly (the inner error is only logged), the shadowing is confusing and could lead to bugs during future maintenance.

♻️ Suggested fix

 			func(key, value interface{}) {
 				p.Logger.V(1).Info("Evicting a Nebius Mysterybox client", "apiDomain", key.(ClientCacheKey).APIDomain)
-				err := value.(mysterybox2.Client).Close()
-				if err != nil {
-					p.Logger.Error(err, "Failed to close Nebius Mysterybox client")
+				closeErr := value.(mysterybox2.Client).Close()
+				if closeErr != nil {
+					p.Logger.Error(closeErr, "Failed to close Nebius Mysterybox client")
 				}
 			})

providers/v1/nebius/common/sdk/mysterybox/grpc_client.go (2)

105-121: Consider adding nil checks for entries in the loop.

Unlike GetSecretByKey which now validates entry.GetData() != nil, GetSecret iterates over payload.GetData() without checking if individual entries or entry.Payload are nil before the type switch. If the gRPC response contains a nil entry, this could panic.

🛡️ Proposed defensive check

 	payloadEntries := make([]Entry, 0, len(payload.Data))
 	for _, entry := range payload.GetData() {
+		if entry == nil {
+			continue
+		}
 		payloadEntry := Entry{
 			Key: entry.Key,
 		}

---

136-139: Use named fields in struct initialization.

Positional struct initialization can silently break if fields are reordered or new fields are added to GrpcClient. Using named fields improves maintainability.

♻️ Suggested change

 	return &GrpcClient{
-		mbox.NewPayloadService(mysteryboxSdk),
-		mysteryboxSdk,
+		PayloadService: mbox.NewPayloadService(mysteryboxSdk),
+		sdk:            mysteryboxSdk,
 	}, nil

apis/externalsecrets/v1/secretstore_nebius_types.go (2)

44-47: Add +optional marker and omitempty to Certificate field.

The Certificate field in NebiusCAProvider has omitempty in its JSON tag but lacks the +optional kubebuilder marker. For consistency with other optional fields in the codebase, consider adding the marker. Additionally, if the CA provider itself is optional and Certificate is the only field, users might create an empty caProvider: {} block. Consider whether this struct should also have validation.

♻️ Suggested change

 // NebiusCAProvider The provider for the CA bundle to use to validate Nebius server certificate.
 type NebiusCAProvider struct {
+	// +optional
 	Certificate esmeta.SecretKeySelector `json:"certSecretRef,omitempty"`
 }

---

55-56: Use // comment style for consistency.

The comment on line 55 uses /* */ style while all other comments in this file use //. Using consistent comment style improves readability.

♻️ Suggested change

-	/* Auth defines parameters to authenticate in Mysterybox */
+	// Auth defines parameters to authenticate in Mysterybox.
 	Auth NebiusAuth `json:"auth"`

config/crds/bases/external-secrets.io_secretstores.yaml (1)

3788-3791: Consider dropping provider-specific CEL validation for auth.

To keep CRD validations consistent across providers, it may be better to rely on runtime validation/documentation instead of adding a provider-specific x-kubernetes-validations rule here.

♻️ Suggested change

-                        x-kubernetes-validations:
-                        - message: either serviceAccountCredsSecretRef or tokenSecretRef
-                            must be set
-                          rule: has(self.serviceAccountCredsSecretRef) || has(self.tokenSecretRef)

Based on learnings: In CRD YAML files under config/crds, avoid provider-specific schema validations (e.g., format: uri or CEL rules) when other providers don’t use them; prioritize consistency across providers.