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

v3 should allow to update docker registry credentials #3467

Conversation

kathap
Copy link
Contributor

@kathap kathap commented Oct 10, 2023

CAPI issue: #3304 Missing v3 feature parity.

PATCH /v3/packages/:guid should allow to update docker registry credentials.

This change adds the possibility to update the docker credentials via PATCH /v3/packages/:guid. After the PATCH the user has to restage the app in order to compile a new droplet with the new credentials.

Copy link
Member

@moleske moleske left a comment

Choose a reason for hiding this comment

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

Can you update the docs to show this new support for username and password?

Otherwise seems good. You could consider adding a test at the controller integration level if you wanted to test the flow the whole way through (this test file.) This would allow you to exercise the whole flow with real components, but some might consider it too much for this change. I can probably be convinced either way

@kathap
Copy link
Contributor Author

kathap commented Oct 11, 2023

Can you update the docs to show this new support for username and password?

Otherwise seems good. You could consider adding a test at the controller integration level if you wanted to test the flow the whole way through (this test file.) This would allow you to exercise the whole flow with real components, but some might consider it too much for this change. I can probably be convinced either way

Sure, documentation will be updated. Maybe I set it to draft, still working on tests and cases where only password or username is updated :)
Maybe also this documentation needs then an update..

@kathap kathap marked this pull request as draft October 11, 2023 09:42
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 18, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@kathap kathap force-pushed the patch-v3-packages-guid-should-allow-to-update-docker-registry-credentials branch 3 times, most recently from 8e558d0 to 43dc1a2 Compare October 19, 2023 10:00
@kathap kathap marked this pull request as ready for review November 2, 2023 10:19
@kathap kathap marked this pull request as draft November 2, 2023 10:22
@kathap kathap marked this pull request as ready for review November 2, 2023 11:44
kathap added a commit to kathap/docs-dev-guide that referenced this pull request Nov 2, 2023
With this PR cloudfoundry/cloud_controller_ng#3467 you do not have to push your app for updating docker credentials, you can also update the latest package and restage your app.
@kathap
Copy link
Contributor Author

kathap commented Nov 6, 2023

Tested the new PATCH, works with a restage after the PATCH, you can update username and password or just one of them.

app/actions/package_update.rb Outdated Show resolved Hide resolved
app/actions/package_update.rb Show resolved Hide resolved
app/actions/package_update.rb Outdated Show resolved Hide resolved
app/controllers/v3/packages_controller.rb Show resolved Hide resolved
app/messages/package_update_message.rb Outdated Show resolved Hide resolved
spec/unit/actions/package_update_spec.rb Outdated Show resolved Hide resolved
CAPI issue: cloudfoundry#3304
Missing v3 feature parity.

PATCH /v3/packages/:guid should allow to update docker registry credentials.

This change adds the possibility to update the docker credentials via
PATCH /v3/packages/:guid
@kathap kathap force-pushed the patch-v3-packages-guid-should-allow-to-update-docker-registry-credentials branch from 3485151 to 98ece64 Compare November 7, 2023 13:54
@philippthun philippthun merged commit 7aeca15 into cloudfoundry:main Nov 8, 2023
7 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.

None yet

4 participants