feat: merge jaas resources feature branch #597
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The AvoidJAASValidator can be used to ensure a resource is not used with JAAS. E.g. juju_access_model should not be used with JAAS as there is a JAAS specific type for that. The RequiresJAASValidator can be used to ensure a resource is only used with JAAS. E.g. new JAAS specific resources should not be used when communicating with a Juju controller.
- avoid leaking connection - reword error messages - make validate function not exported
…ators feat: add jaas validators
Add a generic resource access type that will form the basis for jaas access resources. Add validators for fields including username, model id and group id validators. Add jaasModelAccess type to perform access management to models using the new generic type.
Add missing copyright headers Add missing go docs Add tests for regex validators
feat: add generic jaas access resource
- Don't leak api types outside of the jaas client. - Cleanup tests
Updated godocs
feat: add jaas client
- Add workflow to start JAAS and run tests. - Add logic to skip tests that are not appropriate for JAAS.
juju#561 ## Description This PR adds an integration test workflow to test the provider against a JIMM controller. The full list of changes include: - Add workflow to start JAAS and run tests. - Add logic to skip tests that are not appropriate for JAAS. - Fix offer tests that used a fixed "admin" user in assertions. Fixes: [CSS-6347](https://warthogs.atlassian.net/browse/CSS-6347) ## Type of change - Change in tests (one or several tests have been changed) ## Additional notes The Github action at `canonical/jimm/.github/action/test-server` is ready but because the `canonical/jimm` repo is not public, it is not accessible. The repo will become public very early next week. Marking this PR as a draft until then. [CSS-6347]: https://warthogs.atlassian.net/browse/CSS-6347?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
- 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.
- Remove globals for isJAAS placing them on the sharedClient struct instead. - Improved error messages when Juju client calls fail. - Use template strings in tests.
CSS-9769 Generic access resource crud
- Ensure certain resources cannot be used with JAAS and add tests for this. - Tweak the error messaging based on previous feedback. - Add constructors for JAAS validators.
feat: add "avoid jaas" validator to existing resources
The 5.19 channel has been removed from the lxd snap track list. Use the latest 3 terraform version, 1.7 - 1.9.
juju#578 The 5.19 channel has been removed from the lxd snap track list.
Fixes: juju#466 Juju does not distinguish between null constraints and empty-string constraints. However, null is not considered by terraform to be a computed value, whereas the empty string is. Since the constraints schema marks constraints as a computed field, this means if we insert null into state it will be considered uncomputed & on the next apply terraform will attempt to compute the constraints by setting them. This leads to bugs since subordinate applications cannot have constraints applied. Since we know that subordinate applications cannot have constraints, on Create or Read we can easily 'compute' the constraints ourselves as "".
The cla check verifies that someone contributing to a Canonical project has signed the Canonical CLA agreement
Call full status with a filter of the application name when Reading, this ensures that the returned storage is for the current application only. Updated an application acceptance test to ensure storage isn't written for applications without storage.
Check the pointer before using it. First reported in juju#549 which has been unreproducible. Added a debug message if it can be reproduced other places to get a better understanding.
- Don't use postgres charms that require storage when running offer tests.
Merge main jaas resources
juju#587 ## Description This PR adds the JAAS group access resource. This resource leverages the generic jaas access logic. Partially addresses: [CSS-10638](https://warthogs.atlassian.net/browse/CSS-10638) ## Type of change - Add new resource [CSS-10638]: https://warthogs.atlassian.net/browse/CSS-10638?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
- Use custom parse tag function to handle the fact that applicationoffer tags expect UUIDs.
juju#584 ## Description This PR adds a new resource `juju_jaas_access_offer` building on top of the generic access structure. Note that there is a minor issue with Juju application offer tags. In the Juju names/v4 package, an `applicationoffer` tag was based on an offer URL. In the names/v5 package this changed to an offer UUID. JAAS accepts both but currently the provider does not store the offer UUID (proposed change in juju#583). Until then I've opted to use the Juju names/v4 package to construct application offers based on an offer URL. This PR is based on top of juju#581. See the last commit for the changes. Partially resolves: [CSS-10638](https://warthogs.atlassian.net/browse/CSS-10638) ## Type of change - Add new resource [CSS-10638]: https://warthogs.atlassian.net/browse/CSS-10638?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
juju#591 ## Description ~This PR builds on juju#590. Review only the last commit to see relevant changes.~ This PR introduces a JAAS service account access resource. This defines the permissions towards service accounts in JAAS and leverages on the generic access logic. Partially addresses: [CSS-10638](https://warthogs.atlassian.net/browse/CSS-10638) ## Type of change - Add new resource [CSS-10638]: https://warthogs.atlassian.net/browse/CSS-10638?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
- Special case the tag controller-jimm when parsing tags
juju#590 ## Description ~This PR builds on juju#584.~ This PR introduces a JAAS acccess controller resource. This defines the permissions towards the JAAS controller and leverages on the generic access logic. There is one unique aspect of this resource compared to the other JAAS access resources - this resource has no target object. I.e. A user does not specify a specific controller. This resources defines the access on the JAAS controller itself. Partially addresses: [CSS-10638](https://warthogs.atlassian.net/browse/CSS-10638) ## Type of change - Add new resource [CSS-10638]: https://warthogs.atlassian.net/browse/CSS-10638?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
- Simplify imports by eliminating the need for users to provide the resource type - Add note that client credentials auth is only supported with JAAS
kian99
changed the title
juju#594 ## Description This PR adds documentation for all JAAS resources. Additionally it simplifies imports so that users don't have to specify the resource type in the import. Full changes include: - Added example plans and import scripts for all JAAS resources. - Ran `make docs` to generate new docs. - Updated the description for the "relation" field to point to JAAS docs which contains valid relation values. Fixes: [CSS-10812](https://warthogs.atlassian.net/browse/CSS-10812) ## Type of change - Change existing resource - Other (documentation) [CSS-10812]: https://warthogs.atlassian.net/browse/CSS-10812?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
kian99
force-pushed
the
merge-jaas-resources-to-main
branch
from
0bb0c7b
to
39f1181
Compare
hmlanigan
approved these changes
Sep 26, 2024
|
/merge |
1 similar comment
|
/merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR merges the work from the feature branch
jaas-resourcesinto main. This PR includes #594 which is yet to land.There is one additional piece of work for group data sources that will need to land after the code freeze or in addition to this PR.
Type of change