bindings/go/oci/spec/credentials/identity/v1/oci_registry_identity_test.go (1)

45-48: Also assert Type survives round-trip conversion.

Line 31 initializes Type, but the current assertions only verify auxiliary fields. Adding a Type assertion will catch regressions in typed identity conversion behavior.

✅ Suggested test addition

 	assert.Equal(t, original.Hostname, restored.Hostname)
 	assert.Equal(t, original.Scheme, restored.Scheme)
 	assert.Equal(t, original.Port, restored.Port)
 	assert.Equal(t, original.Path, restored.Path)
+	assert.Equal(t, original.Type, restored.Type)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/go/oci/spec/credentials/identity/v1/oci_registry_identity_test.go`
around lines 45 - 48, Add an assertion that verifies the identity Type survives
round-trip conversion by asserting original.Type equals restored.Type in the
same test that already checks Hostname, Scheme, Port, and Path (look for the
variables original and restored in oci_registry_identity_test.go); update the
assertions block to include assert.Equal(t, original.Type, restored.Type) so
regressions in typed identity conversion are caught.

bindings/go/oci/transformer/credentials_test.go (1)

42-45: Prefer credential key constants over string literals in assertions.

Using ocicredsv1.CredentialKey* constants here will keep tests resilient to key renames.

♻️ Suggested assertion update

-	assert.Equal(t, "user", creds["username"])
-	assert.Equal(t, "pass", creds["password"])
-	assert.Equal(t, "tok", creds["accessToken"])
-	assert.Equal(t, "ref", creds["refreshToken"])
+	assert.Equal(t, "user", creds[ocicredsv1.CredentialKeyUsername])
+	assert.Equal(t, "pass", creds[ocicredsv1.CredentialKeyPassword])
+	assert.Equal(t, "tok", creds[ocicredsv1.CredentialKeyAccessToken])
+	assert.Equal(t, "ref", creds[ocicredsv1.CredentialKeyRefreshToken])

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/go/oci/transformer/credentials_test.go` around lines 42 - 45,
Replace the string literal keys used in the assertions with the ocicredsv1
credential key constants to make tests robust to renames: change accesses of
creds["username"], creds["password"], creds["accessToken"], and
creds["refreshToken"] in the test (where the variable creds is asserted) to use
ocicredsv1.CredentialKeyUsername, ocicredsv1.CredentialKeyPassword,
ocicredsv1.CredentialKeyAccessToken, and ocicredsv1.CredentialKeyRefreshToken
respectively so the assert.Equal calls reference the constant keys.

bindings/go/oci/spec/credentials/v1/oci_credentials_test.go (1)

104-110: Cover the unversioned alias in the registration test.

MustRegisterCredentialType advertises backward compatibility by registering both OCICredentials/v1 and OCICredentials, but this test only exercises the versioned path. Adding an assertion for runtime.NewUnversionedType(OCICredentialsType) would protect the alias contract from regressing unnoticed.

Suggested test addition

 func TestMustRegisterCredentialType(t *testing.T) {
 	scheme := runtime.NewScheme()
 	MustRegisterCredentialType(scheme)

 	obj, err := scheme.NewObject(runtime.NewVersionedType(OCICredentialsType, Version))
 	require.NoError(t, err)
 	assert.IsType(t, &OCICredentials{}, obj)
+
+	obj, err = scheme.NewObject(runtime.NewUnversionedType(OCICredentialsType))
+	require.NoError(t, err)
+	assert.IsType(t, &OCICredentials{}, obj)
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/go/oci/spec/credentials/v1/oci_credentials_test.go` around lines 104
- 110, The test TestMustRegisterCredentialType only verifies the versioned
registration; update it to also exercise the unversioned alias by calling
scheme.NewObject(runtime.NewUnversionedType(OCICredentialsType)) after
MustRegisterCredentialType and assert no error and that the returned object is
of type *OCICredentials; this ensures MustRegisterCredentialType registers both
runtime.NewVersionedType(OCICredentialsType, Version) and the unversioned alias.

bindings/go/oci/transformer/credentials.go (1)

34-35: Return a copy of DirectCredentials.Properties.

This branch leaks the resolver-owned map as-is. Unlike the OCICredentials path, any downstream normalization or augmentation of the returned credentials would mutate the stored DirectCredentials instance and affect later resolutions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/go/oci/transformer/credentials.go` around lines 34 - 35, The
DirectCredentials branch currently returns the resolver-owned map directly (case
*credconfigv1.DirectCredentials: return c.Properties, nil), which leaks internal
state; modify the branch to return a shallow copy of c.Properties instead
(create a new map, copy each key/value from c.Properties into it) so callers can
mutate the returned map without affecting the stored DirectCredentials instance
— mirror the safe behavior used in the OCICredentials path by returning the
cloned map and nil error.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

bindings/go/oci/transformer/credentials_test.go (1)

48-64: 💤 Low value

Consider adding a test for all-empty OCICredentials.

The implementation returns nil, nil when all fields are empty (via if len(result) == 0). This edge case isn't explicitly covered. Optional addition:

func TestResolveCredentialsMap_OCICredentials_AllEmpty(t *testing.T) {
    resolver := &stubResolver{
        typed: &ocicredsv1.OCICredentials{},
    }

    creds, err := resolveCredentialsMap(t.Context(), resolver, runtime.Identity{"type": "OCIRegistry"})
    require.NoError(t, err)
    assert.Nil(t, creds, "all-empty credentials should return nil map")
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/go/oci/transformer/credentials_test.go` around lines 48 - 64, Add a
unit test that verifies resolveCredentialsMap returns nil (not an empty map)
when OCICredentials has all empty fields: create a stubResolver with typed equal
to &ocicredsv1.OCICredentials{} and call resolveCredentialsMap(t.Context(),
resolver, runtime.Identity{"type":"OCIRegistry"}); assert no error and
assert.Nil on the returned creds. Name the test e.g.
TestResolveCredentialsMap_OCICredentials_AllEmpty and keep it alongside the
existing TestResolveCredentialsMap_OCICredentials_SkipsEmptyFields.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes