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

CSS-9769 Generic access resource crud #552

Merged
merged 2 commits into from
Sep 12, 2024

Conversation

kian99
Copy link
Contributor

@kian99 kian99 commented Aug 16, 2024

Description

This PR builds on #561, review from the second commit to skip the changes from that PR.

This PR implements CRUD methods for the generic resource access object.

The generic resource access object constructs tuples based on the user defined set of users/groups/service-accounts that should have a level of access to a particular resource.

Type of change

  • Change existing resource

Additional notes

Addresses CSS-9769

@kian99 kian99 force-pushed the CSS-9769-generic-access-crud branch 3 times, most recently from d0a93a9 to 6ffb42d Compare August 30, 2024 18:24
@kian99 kian99 requested a review from hmlanigan August 30, 2024 18:24
@kian99 kian99 marked this pull request as ready for review September 4, 2024 13:25
@kian99 kian99 force-pushed the CSS-9769-generic-access-crud branch 2 times, most recently from 08b31c9 to 8508652 Compare September 5, 2024 14:30
Copy link
Member

@hmlanigan hmlanigan left a comment

Choose a reason for hiding this comment

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

Please update the commit message with more detail. You're doing more in this commit than implementing CRUD methods for the provider.

internal/juju/client.go Outdated Show resolved Hide resolved
internal/juju/client.go Outdated Show resolved Hide resolved
internal/provider/expect_recreated_resource_test.go Outdated Show resolved Hide resolved
internal/provider/resource_access_generic.go Show resolved Hide resolved
internal/provider/resource_access_generic.go Outdated Show resolved Hide resolved
internal/provider/resource_access_generic.go Outdated Show resolved Hide resolved
internal/provider/resource_access_generic.go Outdated Show resolved Hide resolved
internal/provider/resource_access_generic.go Show resolved Hide resolved
internal/provider/resource_access_jaas_model_test.go Outdated Show resolved Hide resolved
@kian99 kian99 force-pushed the CSS-9769-generic-access-crud branch from 8508652 to 46e2847 Compare September 11, 2024 07:36
- Modify the jaasAccessModelResourceModel struct to avoid embedding
  - Embedding doesn't work with Terraform's reflection.
- Modify the resourceInfo interface to perform plan get/saves on the parent object.
@kian99 kian99 force-pushed the CSS-9769-generic-access-crud branch 3 times, most recently from 11ff056 to 615f980 Compare September 11, 2024 07:58
- Remove globals for isJAAS placing them on the sharedClient struct instead.
- Improved error messages when Juju client calls fail.
- Use template strings in tests.
@kian99 kian99 force-pushed the CSS-9769-generic-access-crud branch from 615f980 to 03f53fb Compare September 11, 2024 10:04
@kian99 kian99 requested a review from hmlanigan September 11, 2024 12:30
Copy link
Member

@hmlanigan hmlanigan left a comment

Choose a reason for hiding this comment

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

LGTM

@kian99
Copy link
Contributor Author

kian99 commented Sep 12, 2024

@hmlanigan Can you please merge this PR, all tests are passing.

@hmlanigan hmlanigan merged commit f008b48 into juju:jaas-resources Sep 12, 2024
29 checks passed
jujubot added a commit that referenced this pull request Sep 17, 2024
#571

## Description

This PR builds on top of #552.

In this PR we implement imports for any resources built on top of the JAAS generic access resource. Imports are enabled via a string of the format `resourceTag:accessLevel`. The concrete resource must also implement an import "hint" to guide users, e.g. the JAAS model access resource hints that the import string must be something like "model-UUID:writer".

Fixes: [CSS-10237](https://warthogs.atlassian.net/browse/CSS-10237)

## Type of change

- Change existing resource (implement imports)


[CSS-10237]: https://warthogs.atlassian.net/browse/CSS-10237?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
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.

3 participants