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

CLI-997: removes all usages of user.organization_id throught the CLI #893

Merged
merged 12 commits into from
Jul 1, 2021

Conversation

zzbennett
Copy link
Contributor

@zzbennett zzbennett commented Jun 18, 2021

Checklist

  1. [CRUCIAL] Is the change for CP or CCloud functionalities that are already live in prod?

    • yes: ok
  2. Did you add/update any commands that accept secrets as args/flags?

    • no: ok

What

We are deprecating user.organization_id in favor of organization.id, which should always be fetched from the auth context or explicitly passed in via parameters. This is for project nexus's initiative to support multiple users per org--we can no longer assume a user belongs to only one org, therefore we cannot rely on the user.organization_id field anymore.

References

Test&Review

I built this locally with a version of cc-structs that does not have user.organization_id defined and found all the places where that field is referenced and replaced them with organization.id, or removed the user.organization_id entirely, since in almost all our APIs the organization id is read from the jwt token and not from explicitly api parameters.

gonna wait for the ci jobs to pass, since this does not introduce any new logic, we just want to avoid regressions

@zzbennett zzbennett requested review from ziru and LucasLyld June 18, 2021 18:06
Copy link
Member

@ziru ziru left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@mtodzo mtodzo left a comment

Choose a reason for hiding this comment

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

lgtm when CI passes

go.mod Outdated
@@ -24,7 +26,7 @@ require (
github.com/confluentinc/cc-structs/kafka/util v0.753.0
github.com/confluentinc/cc-structs/operator v0.753.0
github.com/confluentinc/cc-utils-public v0.1.0
github.com/confluentinc/ccloud-sdk-go-v1 v0.0.79
github.com/confluentinc/ccloud-sdk-go-v1 v0.0.79-0.20210616223458-373e540901a7
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intentional? Looks like this might be reason for the test failure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yes, it is to pull in my dependent ccloud-sdk-go PR here https://github.com/confluentinc/ccloud-sdk-go-v1/pull/147 I wanted to run the ccloud-sdk-go changes through the CLI CI jobs before merging it. I will update the version here though before merging the CLI PR.

go.mod Outdated
github.com/google/gofuzz v1.1.0 // indirect
github.com/google/uuid v1.2.0
github.com/goreleaser/goreleaser v0.162.1
github.com/goreleaser/goreleaser v0.169.0
Copy link
Contributor

Choose a reason for hiding this comment

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

@DABH Is a golreleaser bump ok? Idk i can't remember if this has caused issues in the past

Copy link
Contributor

Choose a reason for hiding this comment

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

In general we should probably just keep it the same if possible. It takes a few extra changes to really upgrade the version...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah did not mean to bump that up. I did run go get goreleaser which must have bumped the version

@zzbennett zzbennett merged commit aedf810 into master Jul 1, 2021
@zzbennett zzbennett deleted the cli-997 branch July 1, 2021 19:01
brianstrauch pushed a commit that referenced this pull request Jul 1, 2021
…893)

* CLI-997: remove all usages of user.organization_id field in the CLI

* regen S3API mock

* bump ccloud-sdk-go-v1 version to pull in latest changes from master branch

* revert goreleaser version change

* fix failing billing test

* bump ccloud-sdk-go-v1 version

* update cc-system-tests version
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