To enable spaces in policy names

[cniackz]

Since mc allows them:

mc admin policy create play "1Password IT" mypolicy.json

[cesnietor]

@cniackz PTAL at the validSave cause the validation you changed checks while typing but during save we run a different logic and the same validation is there.

 const validSave =
    policyName.trim() !== "" &&
    policyName.indexOf(" ") === -1 &&
    policyDefinition.trim() !== "";

[ramondeklein]

Shouldn't we change MinIO/STS instead, because policy names have the following limitation in AWS IAM (source):

Names of users, groups, roles, policies, instance profiles, server certificates, and paths must be alphanumeric, including the following common characters: plus (+), equals (=), comma (,), period (.), at (@), underscore (_), and hyphen (-).

[cniackz]

@cesnietor, you were right—additional changes were needed for this to work completely. I have made all the necessary adjustments and tested them.

[cniackz]

@ramondeklein, that’s a good question. Since this is already supported by mc, I’m unsure how to proceed. I’ve added @ramondeklein and @harshavardhana to help determine the best course of action.

[ramondeklein]

It's hard to change now, because people may already use these names, so disallowing now is hard. We are S3 compatible, not IAM compatible, so deviating from AWS IAM isn't a real issue. Let's be practical and make sure we're compatible with our implementation.

[cniackz]

Understood, @ramondeklein. I’ll leave this change up to you. Please approve and merge if you think it’s fine to proceed, or add any additional comments if you believe we shouldn’t or have a different perspective. Thanks!

[ramondeklein]

I think it's bad to perform validations in the AIStor back-end. These checks should be done by MinIO itself.

[cesnietor]

@cniackz I can't create a new policy the Save button never gets enabled

How it looks:

How it should look when passing validations:

[cesnietor]

Also I noticed that we allow spaces after and before the policy, those I think we should remove.

I couldn't test if those are saved like that or we actually trimmed them (due to the error above) but worth considering. If we trim them after submitting then is fine.

[cniackz]

Why it does not work for you?... I need time to figure this out as it worked for me, we can synch on this later 👍

[cniackz]

@cesnietor, I’m still unsure why it’s not working for you. I retested everything from scratch and was able to create the policy with spaces in the name. Here’s the screenshot:

Would you mind pairing up this afternoon to look into this together? The steps I used for testing are documented in this wiki: How to test console UI.

[cniackz]

Okay, I’ve made all the requested changes, and everything works as expected. Later today, I’ll pair with Nieto to review this and determine if anything else is missing.

[cesnietor]

tested works as expected.

this was addressed.

[bexsoft]

LGTM

[bexsoft]

Can you please help us to review

policy_test.go:237: 
        	Error Trace:	/home/runner/work/console/console/integration/policy_test.go:237
        	Error:      	Not equal: 
        	            	expected: 400
        	            	actual  : 201
        	Test:       	Test_AddPolicyAPI
        	Messages:   	Create Policy - Space in Name Failed

Seems like this test should be reworked, beside of that LGTM. Thank you very much

[cniackz]

Sure Benjamin, let look into that test deeper

[cniackz]

The issue at policy_test.go:237 comes from the Integration Tests with Latest Distributed MinIO (1.23.x)

I will re-run this test to ensure the failure is replicable. Simultaneously, I will create an empty PR to verify if the failure was introduced in this PR.

[cniackz]

Haha, well, it’s obvious just by looking at the log:

=== NAME  Test_AddPolicyAPI
    policy_test.go:237: 
        	Error Trace:	/home/runner/work/console/console/integration/policy_test.go:237
        	Error:      	Not equal: 
        	            	expected: 400
        	            	actual  : 201
        	Test:       	Test_AddPolicyAPI
        	Messages:   	Create Policy - Space in Name Failed

test change was addressed

[cniackz]

Now waiting for the tests to complete. If everything passes, I’ll request another round of reviews to finalize this.

[cniackz]

Ok, @cesnietor and I have reviewed this test, and the failure is unrelated. Therefore, this PR is now ready for review: