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.

Sign up for GitHub

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

Jump to bottom

Refactor secrets to use new URI format and api args #14426

Merged
merged 2 commits into from
Aug 5, 2022
Merged

Refactor secrets to use new URI format and api args #14426

merged 2 commits into from
Aug 5, 2022

Conversation

wallyworld
Copy link
Member

@wallyworld wallyworld commented Aug 4, 2022 •
edited
Loading

Initial refactor of secrets based on new spec. This PR introduces the new URI format with the resulting changes woven through the various api layers. Next PRs will continue the refactoring.

Checklist

  • Code style: imports ordered, good names, simple structure, etc
  • Comments saying why design decisions were made
  • Go unit tests, with comments saying what you're testing
  • [ ] Integration tests, with comments saying what you're testing
  • doc.go added or updated in changed packages

QA steps

boostrap and deploy a charm, eg ubuntu

$ juju exec --unit ubuntu/0 "secret-add --tag foo=bar --description blah secret!"
secret:cbm3aqfnuod28tu36h50
$ juju secrets
URI                          Revision  Rotate  Backend  Age
secret:cbm3aqfnuod28tu36h50         1  never   juju     5 seconds ago  
$ juju exec --unit ubuntu/0 "secret-get secret:cbm3aqfnuod28tu36h50"
secret!

@wallyworld wallyworld added do not merge Even if a PR has been approved, do not merge the PR! 3.0 labels Aug 4, 2022
@wallyworld wallyworld force-pushed the initial-secrets-refactor branch 4 times, most recently from f485fa6 to 11ec44c Compare August 4, 2022 11:25
@wallyworld wallyworld removed the do not merge Even if a PR has been approved, do not merge the PR! label Aug 4, 2022
ycliuhw
ycliuhw approved these changes Aug 5, 2022
View reviewed changes
Copy link
Member

@ycliuhw ycliuhw left a comment

Choose a reason for hiding this comment

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

LGTM ty

@wallyworld
Copy link
Member Author

/merge

@wallyworld
Copy link
Member Author

/merge

1 similar comment
@wallyworld
Copy link
Member Author

/merge

@jujubot jujubot merged commit c33e16f into juju:develop Aug 5, 2022
@manadart manadart mentioned this pull request Aug 15, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ycliuhw ycliuhw ycliuhw approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@wallyworld @ycliuhw @jujubot