Return default value for role alias_name_source instead of empty string

[tsaarni]

Overview

New field alias_name_source was introduced to Role in Vault 1.9.0. If Role was created with older version of Vault, the field is missing from the persisted config in storage. When user sends "Read Role" request to read the stored Role with new Vault version, it will be returned with invalid value "alias_name_source": "". The reason is that the corresponding Go struct gets initialized with empty string when deserializing the stored JSON without the field.

Design of Change

This change returns the default value "serviceaccount_uid" instead of empty string, which makes the returned config data valid (e.g. it can be applied back in "Create Role" request).

Related Issues

Fixes https://github.com/hashicorp/vault-plugin-auth-kubernetes/issues/133

Contributor Checklist

[N/A] Add relevant docs to upstream Vault repository, or sufficient reasoning why docs won’t be added yet [N/A] Add output for any tests not ran in CI to the PR description (eg, acceptance tests) [X] Backwards compatible

[tsaarni]

Cc @benashz

[tomhjp]

I've discussed this internally with the team, and there was some concern about returning data that doesn't match what's in storage. We're planning to open a different PR to make pathRoleCreateUpdate accept an empty string for alias_name_source and treat it as specifying the default value. That way storage will be updated to a valid value, and reads will always reflect the real data. Would that equally solve your issue?

[tsaarni]

Yes, that would solve it as well

[tomhjp]

OK great, sorry for the late change of tack.

[tsaarni]

@tomhjp One question though:

One alternative that I was pondering about while writing #133 was not to return alias_name_source in "Read Role" at all. That applied to this PR #134 would have been following additional change:

diff --git a/path_role.go b/path_role.go
index ea64e69..2f6ed7d 100644
--- a/path_role.go
+++ b/path_role.go
@@ -182,7 +182,6 @@ func (b *kubeAuthBackend) pathRoleRead(ctx context.Context, req *logical.Request
        if role.NumUses > 0 {
                d["num_uses"] = role.NumUses
        }
-       d["alias_name_source"] = aliasNameSourceDefault
        if role.AliasNameSource != "" {
                d["alias_name_source"] = role.AliasNameSource
        }

In that case empty string would not necessarily need to be accepted, and the public documentation would not necessarily need to change. So in a way, maybe impacts are less but it still would fix my problem of reading and re-applying the config.

But for me any approach is completely ok :)