Refactor & replace dashboard Admin API, implement commands for auth, project, organization, and api-key#33
Conversation
… projects, organizations, and api-keys to use the admin API through the Go SDK. These commands are all now exposed at the root level. Adds new functionality to the sdk package to allow instantiating an AdminClient for use in CLI commands. Add new auth group to the root, this group holds the new auth command, along with existing login, logout, target, and whoami commands, which will also live under auth. Add new configure sub-command to auth to support providing client_id and client_secret for authentication. Refactor NewPineconeClientForProjectById to use the SDK admin commands, and add new helper fn getCLIAPIKeyForProject for managing CLI created keys. Update secrets state to handle caching and storing projectId<->apiKey relationships for keys the CLI uses - most of this will be expanded a bit more later as we fully flesh out how we want to handle credentials
|
Claude finished @austin-denoble's task —— View job PR Review Complete ✅I've conducted a comprehensive review of this pull request that refactors the CLI to use the Admin API and Go SDK. Here's my detailed analysis: Review Todo List:
🏗️ Architecture & DesignStrengths:
Areas for improvement:
🔐 Security ReviewConcerns identified:
Recommendations:
🐛 Potential Bugs
🚀 Performance ConsiderationsGood practices:
Potential improvements:
🧪 Test Coverage AssessmentCurrent state:
Critical gaps:
📋 Additional Recommendations
🎯 Priority Issues to AddressHigh Priority:
Medium Priority:
Low Priority:
This is a solid refactoring that modernizes the CLI architecture. The main concerns are around security hardening and test coverage, but the overall design is well-structured and maintainable. |
| } | ||
| } | ||
|
|
||
| func GetTargetOrgId() (string, error) { |
There was a problem hiding this comment.
Moved these functions into this package from command specific packages.
…rget context so we get project & organization ids along with name, fix precedence behavior for --id vs. targeted project in related API key commands
|
Claude finished @austin-denoble's task —— View job PR Review Complete ✅I've conducted a comprehensive review of this pull request that refactors the CLI to use the Admin API and Go SDK. Here's my detailed analysis: Review Todo List:
🏗️ Architecture & DesignStrengths:
Areas for improvement:
🔐 Security ReviewConcerns identified:
Recommendations:
🐛 Potential Bugs
🚀 Performance ConsiderationsGood practices:
Potential improvements:
🧪 Test Coverage AssessmentCurrent state:
Critical gaps:
📋 Additional Recommendations
🎯 Priority Issues to AddressHigh Priority:
Medium Priority:
Low Priority:
This is a solid refactoring that modernizes the CLI architecture. The main concerns are around security hardening and test coverage, but the overall design is well-structured and maintainable. |
…ey create to use ProjectEditor when no roles provided
… use the target project and organization as fallbacks for id arguments where they make sense, more docs cleanup
|
Claude finished @austin-denoble's task —— View job PR Review Complete ✅I've conducted a comprehensive review of this pull request that refactors the CLI to use the Admin API and Go SDK. Here's my detailed analysis: Review Todo List:
🏗️ Architecture & DesignStrengths:
Areas for improvement:
🔐 Security ReviewConcerns identified:
Recommendations:
🐛 Potential Bugs
🚀 Performance ConsiderationsGood practices:
Potential improvements:
🧪 Test Coverage AssessmentCurrent state:
Critical gaps:
📋 Additional Recommendations
🎯 Priority Issues to AddressHigh Priority:
Medium Priority:
Low Priority:
This is a solid refactoring that modernizes the CLI architecture. The main concerns are around security hardening and test coverage, but the overall design is well-structured and maintainable. |
…kages (#39) ## Problem There are two big issues around authentication this PR is trying to address. First, we're not refreshing our oauth user tokens after initially retrieving one on login (`pc auth login`). This means that after 30 minutes, the token is no longer valid and the user will have to log in again. We need to make sure that we're calling for a refreshed token if the local token is expired. There were [changes merged to the auth0-config ](pinecone-io/auth0-config#287 support this for the Pinecone CLI client. Second, the CLI previously relied on the non-public dashboard APIs in order to work with organizations, projects, and API keys. These implementations were handled manually through the `network` and `dashboard` packages. In a [previous PR](#33), the code for working with admin resources was mostly replaced when implementing commands for `project`, `organization`, and `api-key`. However, we are still using dashboard APIs for the `login` and `target` flows. Additionally, because of how our systems currently work with authentication, the CLI needs additional business logic for working with the control and data plane APIs when the user has authenticated via a user token (`pc auth login`) or a service account (`pc auth configure --client-id --client-secret`). Because there are limitations on where these tokens can be used across APIs surfaces, the CLI needs mechanisms for creating its own API keys, and allowing users to "store" keys for specific projects. This in turn means we need to expose ways for users to manage these locally stored keys themselves. If you're curious or have any feedback, I've captured [notes and thoughts here](https://www.notion.so/CLI-Auth-Credentials-Paths-2632b1c5593b80b5bb6cf7d8e55039ac?source=copy_link) while working on an approach that will work for the CLI with our current authentication structure. ## Solution While it's a large PR, there haven't been any breaking changes made to the existing CLI interface. A lot of the credentials management logic is mostly abstracted from the user, but there are a number of new flags for existing commands that allow better ease-of-use when working with resources. Additionally, there has been new sub-commands added under `pc auth` to allow listing and pruning "local keys". I'll try and break things into sections below for review and testing focus. ### Supporting oauth user token refresh Previously, the CLI would get an oauth user token on `pc auth login`, store it once using `secrets.OAuth2Token.Set()`, and that was it. Since any code paths that needed the token for building the SDK would just pull it directly out of `secrets.OAuth2Token.Get()`, there was never any token refresh happening when expiry was reached. It's also not great that we have a lot of logic directly getting and setting against our Viper storage (local config files). This will lead to race conditions if multiple calls are made getting/setting within the same process, or if there are concurrent processes accessing these things (IE: a user having multiple shells opens running things using the CLI). Some of this is more forward-thinking, but I tried to be aware of it as I worked on an implementation to handle refreshing tokens. Ultimately, I decided on writing a TokenManager singleton (`sync.Once`), that exposes a method `Token` for anywhere that needs access to the user token. This effectively acts as a wrapper for the secrets package where I've added a `sync.RWMutex` along with getters and setters for the underlying Viper `MarshaledProperty`. I've also made the property itself local so it's not directly exported from the secrets package. We should most likely do this for anything we're storing locally, but I've held off refactoring everything for now. - Implement `TokenManager`. Add a public Token() function to the `oauth` package which returns TokenManager.Token(). Implement logic for refreshing when needed. Implement custom HTTP logic for calling for a refresh in `refreshTokenWithOrg`. We need to do this because we need to pass `orgId` in the body or the refreshed token will default to a different org, and the Go `oauth2` package doesn't inherently support this. - Rename the `oauth2` package to `oauth` in utils. - Replace all instances of calling `secrets.OAuth2Token.Get()` directly with `oauth.Token()`. ### Handling multiple types of "auth" There are a lot of changes and general refactoring in this PR to smooth out the experience of having several different forms of authentication that the user could be working with. The current primary forms of authentication are: - User Token (`pc auth login`) - Service Account Credentials (`pc auth configure --client-id --client-secret`) - Global API Key (`pc config set-api-key` or `pc auth configure --api-key`) | | Admin API access | Control Plane access | Data Plane access | v3 Data Plane Access | | --- | --- | --- | --- | --- | |User Token | ✅ | ✅ (with `X-Project-Id` header) | ✅ (with `X-Project-Id` header) | ❌ | |Service Account | ✅ | ❌ | ❌ | ❌ | |Global API Key | ❌ | ✅ | ✅ | ✅ | Because of the variation in levels of access currently in our auth systems, the CLI needs to allow users to create new keys and store them locally for later use. Additionally, the CLI needs to be able to create its own managed keys if the user does not explicitly create keys before working with the control or data plane APIs. Currently, the console does a form of this by generating a "default" API key when you first log in. The console has access to unstable APIs that allow them to more easily work with API keys generally, so we have to make some concessions in the CLI where we're relying on the public admin APIs. - "Global" API key set via `pc auth configure --api-key` or `pc config set-api-key` will override the target organization, project, and any internally managed keys. This applies to operations involving the control and data plane (index, collections, etc). **NOTE:** We most likely need better documentation and messaging around this behavior. Any suggestions would be great. - Add `pc auth clear --global-api-key --service-account` to allow users a way to remove any configurations they've provided. - Add `pc auth status` to print out a detailed summary of the current authentication credentials that are configured. - User Token & Service Account credentials are now mutually exclusive. If you call `pc auth configure --client-id --client-secret` after `pc auth login` and vice versa, it will log you out or clear the previously set credentials. This will also reset the current Organization and Project target because service accounts are scoped to a single organization. - After calling `pc auth configure --client-id --client-secret`, the CLI will use the admin API to fetch organization and project information, allowing the user to accurately target their org and project. Storing "managed" keys locally is a part of handling auth, and is explained a bit more below. ### Managed Keys (locally stored `map[project_id]:ManagedKey`) Because of limitations described previously, the CLI needs to have some concept of maintaining API keys for projects locally that can be reused across actions. There's no way to fetch an API keys value after the initial create call through the public Admin API. There's also no way to determine a keys organization or project if we just have access to the raw value, such as when a global API key is configured. There are also a lot of possible edge cases around an API key being used and then deleted outside of the CLI context, or manually by a user through the CLI. My approach to resolving some of these issues and presenting a streamlined experience within the CLI is to introduce the concept of `ManagedKeys`. These are keys that the CLI is essentially managing in state, and they can either be created manually by a user through the CLI, or the CLI will create one to manage itself if the user has not created one explicitly. The map of project keys is stored locally in a new `MarshaledProperty` in the `secrets` Viper store. Check `secrets.go` to see the implementation of that along with the struct definition for `ManagedKey`. The logic for creating "cli-managed" keys is in the `sdk` package, and is tied to when we instantiate the `Pinecone` Go SDK. If the CLI needs to create its own key for management, this will be seamless to the user if they call something that requires it, like `pc index list`. In order to manage keys that are stored locally, there are new flags and commands exposed in a few places. - `pc api-key create --name "your-key-name" --store` - the `--store` flag is new, telling the CLI to store this new key for the project it was created in locally. If there was a previously cli-managed key stored, it will be cleaned up. - `pc auth local-keys list` - this will list all the project keys the CLI is maintaining locally in state. - `pc auth local-keys prune --dry-run --skip-confirmation` - this prune command allows cleaning up locally maintained keys. It has flags for filtering what gets deleted: `--dry-run` will output planned changes without applying anything, `--skip-confirmation` will skip the deletion confirmation, `--id` allows cleaning up only one project, and `--origin` allows cleaning up user-created keys, cli-created keys, or all keys. - `pc api-key delete` - this command will now check to see if you're deleting the currently stored key for the project, and remove the locally cached key if you do. I don't think this approach is perfect, but I think for right now it gives us a pretty good overall experience while exposing options for managing your current state. Definitely open to feedback on this approach, and I think things will be evolving more as we move forward. ### Removing `dashboard` and `network` packages These packages are no longer needed due to the above changes, and we can safely remove all dependencies on the console admin API. If you read all this, thank you! ## Type of Change - [ ] Bug fix (non-breaking change which fixes an issue) - [X] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update - [ ] Infrastructure change (CI configs, etc) - [ ] Non-code change (docs, etc) - [ ] None of the above: (explain here) ## Test Plan The simplest way to test these changes locally are going to be checking out this branch (`adenoble/implement-credentials-logic`) locally, and running goreleaser to build the binary. Then just run off of that as you normally would. ```bash $ git checkout adenoble/implement-credentials-logic $ goreleaser build --single-target --snapshot --clean # run commands against the binary $ ./dist/pc_darwin_arm64/pc auth login ``` I would recommend testing using each authentication approach. Using `pc auth status` and `pc target --show` to double check which organization and project you're targeting will be really helpful for validating things. When doing things like swapping your targeted project, you'll want to follow up with a control plane call like `pc index list`. Here's an example of a testing flow I've used locally to get started: ```bash $ ./dist/pc_darwin_arm64/pc auth login [SUCCESS] Logged in as austin.d@pinecone.io. Defaulted to organization ID: -NF9kuQiPWth9QLPg5NS [INFO] Target org set to pinecone-official. [INFO] Target project set cmek-integration-tests. $ ./dist/pc_darwin_arm64/pc auth status ATTRIBUTE VALUE Authentication Mode user_token Global API Key UNSET Service Account Client ID UNSET Service Account Client Secret UNSET Token Expiry 2025-09-23T20:33:40-04:00 Token Time Remaining 29m13s Token Scope openid profile email offline_access Token Organization ID -NF9kuQiPWth9QLPg5NS Environment production $ ./dist/pc_darwin_arm64/pc target --org "SDK Testing" --project pinecone-cli [SUCCESS] Target org updated to SDK Testing [SUCCESS] Target project updated to pinecone-cli # list indexes in this project ./dist/pc_darwin_arm64/pc index list # show local-keys ./dist/pc_darwin_arm64/pc auth local-keys list # target a new project, list indexes, and check keys again ./dist/pc_darwin_arm64/pc target --project austin-dev ./dist/pc_darwin_arm64/pc index list ./dist/pc_darwin_arm64/pc auth local-keys list # test out pruning ./dist/pc_darwin_arm64/pc auth local-keys --prune --skip-confirmation --origin cli # configure a service account and check you're logged out ./dist/pc_darwin_arm64/pc auth configure --client-id "your-id" --client-secret "your-secret" ./dist/pc_darwin_arm64/pc auth status # list projects and make sure targeting still works for projects the service account has access to ./dist/pc_darwin_arm64/pc project list ./dist/pc_darwin_arm64/pc target --project my-project # etc ``` --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1211152838998841
Problem
We need to support admin API functionality in the CLI. Previously, there were commands implemented for working with projects, organizations, and API keys. The original implementation for these features in 2024 used a custom implementation of the Pinecone dashboard APIs which can be found in the
dashboardpackage: https://github.com/pinecone-io/cli/tree/main/internal/pkg/dashboardThese commands were removed from the CLI at that point because we couldn't rely on a custom implementation of the dashboard APIs, and we also wanted to limit their usage to the dashboard itself. Ultimately, for maintainability moving forward we would want to rely on specced and versioned APIs, supported through the Go SDK.
Solution
This PR adds several new commands to the root level of the CLI. The help documentation for each command can be checked by providing the
--help | -hflags at any level. The commands are focused on authorization, and admin resource management:auth- groups commands related to logging in and out, setting the target context, and providing service account details.project- allows working with CRUD operations for projects.api-key- allows working with CRUD operations for API keys within projects.organization- allows for describing, updating, listing, and deleting - we don't support organization create via the API yet.For reviewing the command functionality specifically, look in the folders under
internal/pkg/cli/command/to see where commands and their sub-commands are grouped. The command names should correspond with the top level folders.To support adding these new commands leveraging the Go SDK, there were other changes that needed to be made to the CLI:
client_idandclient_secretfor a service account. Since the number of commands related to working with credentials has grown, I grouped things under theauthcommand mentioned above. This included aliasing the existinglogin,logout,target, andwhoamicommands under the newauthcommand. They are still available at the root. The commands with complex run logic (such aslogin) have had the internal functions extracted into a Run function which can be used in multiple places (see:internal/pkg/utils/login/login.go).internal/pkg/utils/sdk/client.goto support instantiating apinecone.AdminClientto be used for admin operations. There were also changes needed toNewPineconeClientForProjectById, primarily adding a new helper functiongetCLIAPIKeyForProject, which allows the CLI to manage its own API keys for each project it has access to. Since we no longer allow fetching API key secrets after thecreateflow, the CLI needs some way to track it's own keys internally, and use them for non-admin operations. This is similar to what the console does when creating an initial API key for use.internal/pkg/utils/configuration/secrets/secrets.go) to storeClientId,ClientSecret, andProjectAPIKeys. TheProjectAPIKeysstore is the one which handles mapping project <-> CLI-specific API key so that the CLI can work with control and data plane operations within that project as needed.Example help output for root + new commands:
--helpOUTPUTpc --helppc auth --helppc project --helppc organization --helppc api-key --helpType of Change
Test Plan
The easiest way to test this is probably to clone the repo, install goreleaser, and build the binary locally. See "Building the CLI" in the CONTRIBUTING.md file.