minio / mc

Unix like utilities for object store
https://min.io/download
GNU Affero General Public License v3.0
2.86k stars 548 forks source link

set-json to fail on custom policies without .json #4745

Closed r-scheele closed 11 months ago

r-scheele commented 12 months ago

Fix set-json command to fail on custom policies without .json

This commit addresses the bug where the set-json command incorrectly succeeds for custom policies even when the policy file doesn't have a .json extension. The fix ensures that the command fails as expected, prompting the user to use the correct file.

./mc anonymous set-json myminio/mydata ~/test/anonymous.json

Now, set-json will return an error message if the provided policy file is not a .json, aligning the behavior with the documented functionality.

Related to the discussion with @kannappanr

r-scheele commented 12 months ago

File extension shouldn't really matter IMO.

The previous code only validates the file ext, so i think only .json policies are valid, i've added more validations asides the file ext and better error handling that prompted the discussion with @kannappanr. PTAL @klauspost @harshavardhana

File extension shouldn't really matter IMO.

harshavardhana commented 11 months ago

File extension shouldn't really matter IMO.

The previous code only validates the file ext, so i think only .json policies are valid, i've added more validations asides the file ext and better error handling that prompted the discussion with @kannappanr. PTAL @klauspost @harshavardhana

File extension shouldn't really matter IMO.

the file extension check can be relaxed though.

r-scheele commented 11 months ago

File extension shouldn't really matter IMO.

The previous code only validates the file ext, so i think only .json policies are valid, i've added more validations asides the file ext and better error handling that prompted the discussion with @kannappanr. PTAL @klauspost @harshavardhana

File extension shouldn't really matter IMO.

the file extension check can be relaxed though.

Do you mean I should remove it?

harshavardhana commented 11 months ago

File extension shouldn't really matter IMO.

The previous code only validates the file ext, so i think only .json policies are valid, i've added more validations asides the file ext and better error handling that prompted the discussion with @kannappanr. PTAL @klauspost @harshavardhana

File extension shouldn't really matter IMO.

the file extension check can be relaxed though.

Do you mean I should remove it?

Yes @r-scheele