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-10237 generic access imports #571

Merged
merged 2 commits into from
Sep 17, 2024

Conversation

kian99
Copy link
Contributor

@kian99 kian99 commented Sep 11, 2024

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

Type of change

  • Change existing resource (implement imports)

@kian99 kian99 requested a review from hmlanigan September 11, 2024 10:59
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.

a question, and waiting for #552 to land.

resp.Diagnostics.AddError(
"ImportState Failure",
fmt.Sprintf("Malformed Import ID %q, "+
"please use format '<resourceTag>:<access>' e.g. %s", IDstr, a.targetResource.ImportHint()),
Copy link
Member

Choose a reason for hiding this comment

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

question: What is this going to look like in real life? I suggest adding a file: examples/resources/juju_access_jaas_model/import.sh, with the details. You'll need to add these files eventually.

question: what is a "<resourceTag>"? Suggest using a format provided by each type, rather than a generic "resourceTag" indicator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed '<resourceTag>:<access>' so now the message is just please use format %q where the %q is replaced with the import hint.
In the case of a model the import hint is "model-<UUID>:<access-level>"

internal/provider/resource_access_generic.go Show resolved Hide resolved
@kian99 kian99 requested a review from hmlanigan September 12, 2024 06:59
@kian99 kian99 force-pushed the CSS-10237-generic-access-imports branch 2 times, most recently from 26ff384 to 034adb4 Compare September 16, 2024 06:55
@kian99 kian99 force-pushed the CSS-10237-generic-access-imports branch from 034adb4 to b6bd624 Compare September 17, 2024 09:24
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

@hmlanigan
Copy link
Member

/merge

@jujubot jujubot merged commit b3a6209 into juju:jaas-resources Sep 17, 2024
29 of 30 checks passed
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