Skip to content

Commit

Permalink
Fix broken oauth login flow (#15)
Browse files Browse the repository at this point in the history
## Problem
Currently, when you try and use the `pinecone login` command the login
fails after confirming authentication in your browser because of a
panic:

![Screenshot 2024-08-16 at 4 35
13 PM](https://github.com/user-attachments/assets/762cb70c-0d0f-4ba2-b764-1935d85420f4)

We're unsafely accessing a slice. Our decoding of the JSON responses
from the `dashboard` API are failing because the shape of some responses
has changed.

We've had several people run into this issue: 
- https://pinecone-io.slack.com/archives/C06V9RSCBLG/p1723806488866219
- https://pinecone-io.slack.com/archives/C06V9RSCBLG/p1721693472202859

## Solution
I looked at the `dashboard` repo to try and figure out what changed
along with directly dumping the response bodies in the CLI locally. It
seems the json naming and structure of how `organizations` and
`projects` are returned has changed slightly, and projects are no longer
explicitly nested under organizations.

Specifically, this value in the response is now `newOrgs` rather than
`organizations`:
https://github.com/pinecone-io/dashboard/blob/f31f9b126781adf6bf30eb5f137cc0d983ba691b/api-server/functions/src/api/dashboard/organizations/organizations.router.ts#L48

It also seems like there was a `globalProject` field being returned
inside of each `project` previously, and that's no longer the case.
Looks like the fields were flattened into `project`.

- Get rid of `GlobalProject` type and move fields into `Project` which
is what we get back from the `/organizations` functions now.
- Update the json marshaling in `OrganizationResponse` to
``json:"newOrgs"`` to properly decode the response.
- Add logic to re-nest `Projects` inside of `Organizations` when the
`ListOrganizations` response is returned.

### Follow Up
Our reliance on the `dashboard` APIs is somewhat brittle because we're
not always aware when changes to the API interface happen. We need to
improve this process moving forward, and work to make the CLI more
robust in how it handles failures.

## Type of Change
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] 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
You should be able to pull this branch down, use `goreleaser` to build
the CLI locally, and then attempt logging in:

```
goreleaser build --single-target --snapshot --clean
./dist/pinecone_darwin_arm64/pinecone login
```
Here's the testing flow from my end:
Login:
![Screenshot 2024-08-16 at 4 45
03 PM](https://github.com/user-attachments/assets/ba15ad3a-926e-4eb3-8432-7a570bcbcce1)

Target Organization / Project:
![Screenshot 2024-08-16 at 4 45
21 PM](https://github.com/user-attachments/assets/fdfd38cd-ba80-485e-907a-721bf806379f)

List Indexes:
![Screenshot 2024-08-16 at 4 45
28 PM](https://github.com/user-attachments/assets/78471764-970b-4f95-a86e-dde1582991ac)


---
- To see the specific tasks where the Asana app for GitHub is being
used, see below:
  - https://app.asana.com/0/0/1207873887567497
  • Loading branch information
austin-denoble authored Aug 16, 2024
1 parent 51bbc6b commit 81f7555
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 27 deletions.
2 changes: 1 addition & 1 deletion internal/pkg/cli/command/project/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func NewCreateProjectCmd() *cobra.Command {
msg.FailMsg("Failed to create project %s\n", style.Emphasis(options.name))
exit.Error(pcio.Errorf("Create project call returned 200 but with success=false in the body%s", options.name))
}
msg.SuccessMsg("Project %s created successfully.\n", style.Emphasis(proj.GlobalProject.Name))
msg.SuccessMsg("Project %s created successfully.\n", style.Emphasis(proj.Project.Name))
},
}

Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/cli/command/project/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func printTable(projects []dashboard.Project) {
pcio.Fprint(writer, header)

for _, proj := range projects {
values := []string{proj.GlobalProject.Id, proj.Name}
values := []string{proj.Id, proj.Name}
pcio.Fprintf(writer, strings.Join(values, "\t")+"\n")
}
writer.Flush()
Expand All @@ -127,7 +127,7 @@ func printTableAll(orgs *dashboard.OrganizationsResponse) {

for _, org := range orgs.Organizations {
for _, proj := range org.Projects {
values := []string{org.Id, org.Name, proj.Name, proj.GlobalProject.Id}
values := []string{org.Id, org.Name, proj.Name, proj.Id}
pcio.Fprintf(writer, strings.Join(values, "\t")+"\n")
}
}
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/cli/command/target/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func NewTargetCmd() *cobra.Command {
}
state.TargetProj.Set(&state.TargetProject{
Name: proj.Name,
Id: proj.GlobalProject.Id,
Id: proj.Id,
})
}

Expand Down Expand Up @@ -260,7 +260,7 @@ func postLoginSetupTargetProject(orgs *dashboard.OrganizationsResponse, targetOr
} else if len(org.Projects) == 1 {
state.TargetProj.Set(&state.TargetProject{
Name: org.Projects[0].Name,
Id: org.Projects[0].GlobalProject.Id,
Id: org.Projects[0].Id,
})
return org.Projects[0].Name
} else {
Expand All @@ -274,7 +274,7 @@ func postLoginSetupTargetProject(orgs *dashboard.OrganizationsResponse, targetOr
if proj.Name == projectName {
state.TargetProj.Set(&state.TargetProject{
Name: proj.Name,
Id: proj.GlobalProject.Id,
Id: proj.Id,
})
return proj.Name
}
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/dashboard/keys_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const (
URL_GET_API_KEYS = "/v2/dashboard/projects/%s/api-keys"
)

func GetApiKeys(project GlobalProject) (*KeyResponse, error) {
func GetApiKeys(project Project) (*KeyResponse, error) {
return GetApiKeysById(project.Id)
}

Expand Down
40 changes: 28 additions & 12 deletions internal/pkg/dashboard/organizations_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ const (
)

type OrganizationsResponse struct {
Organizations []Organization `json:"organizations"`
Organizations []Organization `json:"newOrgs"`
Projects []Project `json:"projects"`
}

type Organization struct {
Expand All @@ -20,16 +21,11 @@ type Organization struct {
}

type Project struct {
Id string `json:"id"`
Name string `json:"name"`
GlobalProject GlobalProject `json:"globalProject"`
}

type GlobalProject struct {
Id string `json:"id"`
Name string `json:"name"`
Quota string `json:"quota"`
IndexQuota string `json:"indexQuota"`
Id string `json:"id"`
Name string `json:"name"`
OrganizationId string `json:"organization_id"`
Quota string `json:"quota"`
IndexQuota string `json:"index_quota"`
}

func ListOrganizations() (*OrganizationsResponse, error) {
Expand All @@ -42,16 +38,36 @@ func ListOrganizations() (*OrganizationsResponse, error) {
if err != nil {
return nil, err
}
for _, org := range resp.Organizations {

orgToProjectsMap := make(map[string][]Project)

// Organize projects into orgs to match the older data structure
for _, project := range resp.Projects {
if projects, ok := orgToProjectsMap[project.OrganizationId]; ok {
projects = append(projects, project)
orgToProjectsMap[project.OrganizationId] = projects
} else {
orgToProjectsMap[project.OrganizationId] = []Project{project}
}
}

// Modify the response to nest projects under their orgs
for i := range resp.Organizations {
org := &resp.Organizations[i]

log.Trace().
Str("org", string(org.Name)).
Msg("found org")

org.Projects = orgToProjectsMap[org.Id]

for _, proj := range org.Projects {
log.Trace().
Str("org", string(org.Name)).
Str("project", proj.Name).
Msg("found project in org")
}
}

return resp, nil
}
4 changes: 2 additions & 2 deletions internal/pkg/dashboard/projects_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ type CreateProjectRequest struct {
}

type CreateProjectResponse struct {
Success bool `json:"success"`
GlobalProject GlobalProject `json:"globalProject"`
Success bool `json:"success"`
Project Project `json:"globalProject"`
}

func CreateProject(orgId string, projName string, podQuota int32) (*CreateProjectResponse, error) {
Expand Down
12 changes: 6 additions & 6 deletions internal/pkg/dashboard/projects_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,24 @@ import (
"github.com/pinecone-io/cli/internal/pkg/utils/pcio"
)

func GetProjectByName(orgName string, projName string) (*GlobalProject, error) {
func GetProjectByName(orgName string, projName string) (*Project, error) {
orgs, err := ListOrganizations()
if err != nil {
return nil, err
}
for _, org := range orgs.Organizations {
if org.Name == orgName {
for _, proj := range org.Projects {
if proj.GlobalProject.Name == projName {
return &proj.GlobalProject, nil
if proj.Name == projName {
return &proj, nil
}
}
}
}
return nil, error(pcio.Errorf("project name %s not found in organization %s", projName, orgName))
}

func GetProjectById(orgId string, projId string) (*GlobalProject, error) {
func GetProjectById(orgId string, projId string) (*Project, error) {
orgs, err := ListOrganizations()
if err != nil {
return nil, err
Expand All @@ -30,8 +30,8 @@ func GetProjectById(orgId string, projId string) (*GlobalProject, error) {
for _, org := range orgs.Organizations {
if org.Id == orgId {
for _, proj := range org.Projects {
if proj.GlobalProject.Id == projId {
return &proj.GlobalProject, nil
if proj.Id == projId {
return &proj, nil
}
}
}
Expand Down

0 comments on commit 81f7555

Please sign in to comment.