-
Notifications
You must be signed in to change notification settings - Fork 123
Add elasticstack_elasticsearch_security_role_mapping resource #148
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
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
dab7e80 to
05e5460
Compare
05e5460 to
728ab6f
Compare
|
jenkins test this |
tobio
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should include role_templates in the resource. Otherwise this looks great, thanks!
| } | ||
|
|
||
| func (a *ApiClient) GetElasticsearchRoleMapping(ctx context.Context, roleMappingName string) (*models.RoleMapping, diag.Diagnostics) { | ||
| req := a.es.Security.GetRoleMapping.WithName(roleMappingName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we chain the WithContext call here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's not possible...!
| Type: schema.TypeString, | ||
| Required: true, | ||
| DiffSuppressFunc: utils.DiffJsonSuppress, | ||
| Description: "A list of mustache templates that will be evaluated to determine the role names that should granted to the users that match the role mapping rules. This matches fields of users, rules can be grouped into `all` and `any` top level keys.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This description is for role_templates which is missing from the resource schema.
|
Also needs a change log entry here :) |
|
@k-yomo thanks for the groundwork here. Just wanted to check if you're still able to continue on with this one? If not are you ok with me adding the final changes to support |
|
@tobio |
|
jenkins test this |
|
jenkins test this |
|
@dimuon are you able to take a look at the changes in this one? |
I'll take a look |
dimuon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| --- | ||
| # generated by https://github.com/hashicorp/terraform-plugin-docs | ||
| page_title: "elasticstack_elasticsearch_security_role_mapping Resource - terraform-provider-elasticstack" | ||
| subcategory: "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be under Security
Resolves #47