Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow colons in policy name #496

Merged
merged 2 commits into from
Aug 4, 2023

Conversation

sdejong629
Copy link
Contributor

@sdejong629 sdejong629 commented Aug 4, 2023

Description

Policy names are not allowed to contain a colon (:) in the name. This is allowed by the Minio webinterface and is used by SSO to reference claims.

Steps to Reproduce

  1. Add a policy resource
resource "minio_iam_policy" "test" {
  name = "urn:test:bucketname"
  policy= <<EOF
{
  "Version":"2012-10-17",
  "Statement": [
    {
      "Sid":"ListAllBucket",
      "Effect": "Allow",
      "Action": ["s3:PutObject"],
      "Principal":"*",
      "Resource": "arn:aws:s3:::*/*"
    }
  ]
}
EOF
}
  1. Run a terraform plan
  2. Results in error:
Error: "name" must match [\w+=,.@-]
│
│   with minio_iam_policy.test,

Expected behavior:
The resource should be created with the use of colons

Actual behavior:
The policy resource is not created, but the error occurs

Reproduces how often: 100%

Versions

1.17.1

Additional Information

Minio allows the creation of policies using the colon in the web interface. Not really sure why this limitation has been put in the terraform module.

Allows the use of colons in the name of a policy
@felladrin
Copy link
Collaborator

felladrin commented Aug 4, 2023

I think this rule was based on these lines from terraform-provider-aws.
But I couldn't find any reference to this rule on the minio repository, so I'm willing to approve the change.

Copy link
Collaborator

@felladrin felladrin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing it up, @sdejong629! And congrats for your first contribution to this repo! 🎉

@felladrin felladrin merged commit 02df820 into aminueza:master Aug 4, 2023
2 checks passed
szinn referenced this pull request in szinn/k8s-homelab Aug 4, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [minio](https://registry.terraform.io/providers/aminueza/minio)
([source](https://togithub.com/aminueza/terraform-provider-minio)) |
required_provider | patch | `1.17.1` -> `1.17.2` |

---

### Release Notes

<details>
<summary>aminueza/terraform-provider-minio (minio)</summary>

###
[`v1.17.2`](https://togithub.com/aminueza/terraform-provider-minio/releases/tag/v1.17.2)

[Compare
Source](https://togithub.com/aminueza/terraform-provider-minio/compare/v1.17.1...v1.17.2)

#### What's Changed

- Enable the use of colons in policy names by
[@&#8203;sdejong629](https://togithub.com/sdejong629) in
[https://github.com/aminueza/terraform-provider-minio/pull/496](https://togithub.com/aminueza/terraform-provider-minio/pull/496)

#### New Contributors

- [@&#8203;sdejong629](https://togithub.com/sdejong629) made their first
contribution in
[https://github.com/aminueza/terraform-provider-minio/pull/496](https://togithub.com/aminueza/terraform-provider-minio/pull/496)

**Full Changelog**:
aminueza/terraform-provider-minio@v1.17.1...v1.17.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4zMy4zIiwidXBkYXRlZEluVmVyIjoiMzYuMzMuMyIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: repo-jeeves <106431701+repo-jeeves[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants