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

Implement shared credentials across gridscale tools (#136) #153

Closed
wants to merge 19 commits into from
Closed

Implement shared credentials across gridscale tools (#136) #153

wants to merge 19 commits into from

Conversation

JanLukasGithub
Copy link
Contributor

@JanLukasGithub JanLukasGithub commented Jun 8, 2022

  1. Changes the config path from /gscloud/config.yaml to /gridscale/config.yaml and
  2. Changes the config contents from
accounts:
- name: default
  userId: xyz
  token: xyz
  url: https://api.gridscale.io
- name: somthing-else
  userId: xyz
  token: xyz
  url: https://api.gridscale.io

to

projects:
- name: default
  userId: xyz
  token: xyz
  url: https://api.gridscale.io
- name: somthing-else
  userId: xyz
  token: xyz
  url: https://api.gridscale.io

as proposed in #136

Changes the name of --account flag and GRIDSCALE_ACCOUNT environment variable to --project and GRIDSCALE_PROJECT respectively for consistency reasons

There will be no compatibility to GRIDSCALE_ACCOUNT as it wasn't part of the last release, but --account will still be usable, though deprecated and lower priority than --project

The old config path/contents can still be loaded, but the user will be warned of deprecation

gscloud move-config will be introduced. copying and converting the old config to the new one

runtime/config.go Outdated Show resolved Hide resolved
@nvthongswansea
Copy link
Member

@JanLukasGithub this is gonna be a breaking change.

@JanLukasGithub
Copy link
Contributor Author

@JanLukasGithub this is gonna be a breaking change.

That's why I asked

Should I implement a tool to copy the old config to the new location, if the old but not the new one is present, or something along those lines?

And yes this should only be pulled for the next major release (The issue has milestone v0.12.0)

@nvthongswansea
Copy link
Member

@JanLukasGithub this is gonna be a breaking change.

That's why I asked

Should I implement a tool to copy the old config to the new location, if the old but not the new one is present, or something along those lines?

And yes this should only be pulled for the next major release (The issue has milestone v0.12.0)

Yep, a migration will be needed for the new config.

@JanLukasGithub JanLukasGithub marked this pull request as draft June 10, 2022 11:30
@bkircher bkircher self-assigned this Jun 14, 2022
Copy link
Contributor

@bkircher bkircher left a comment

Choose a reason for hiding this comment

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

Looks OK from my side. Wouldn't bother much with automatically moving the old config to the new gridscale/config.yaml path, but it's fine. What is missing is a good Changelog entry. So that users can immediately see the change and act accordingly. Updating the binary is an active step anyway.

README.md Outdated Show resolved Hide resolved
@JanLukasGithub
Copy link
Contributor Author

Should I put the changelog as v0.11.1 or as v0.12.0? I think v0.12.0 is more adequate for the amount and impact of the changes

@bkircher
Copy link
Contributor

@JanLukasGithub Agree, v0.12.0 makes more sense.

@bkircher
Copy link
Contributor

Looks great! Everything looks ready from my side

@JanLukasGithub JanLukasGithub marked this pull request as ready for review June 17, 2022 08:01
@JanLukasGithub
Copy link
Contributor Author

Rebased

@bkircher
Copy link
Contributor

Picked into master branch

@bkircher bkircher closed this Jun 30, 2022
@JanLukasGithub JanLukasGithub deleted the shared-credentials branch June 30, 2022 10:18
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