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.

Sign up for GitHub

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

Bugfix/2935 lakectl bug on not found html #2966

Merged
merged 10 commits into from
Feb 28, 2022
Merged

Bugfix/2935 lakectl bug on not found html #2966

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
855e9ce
avoiding redirect automatically
idanovo Feb 24, 2022
704cacd
Changed all CLI commands to fail on unexpected status code returned
idanovo Feb 24, 2022
3066a9c
Format fixes
idanovo Feb 24, 2022
574c765
Fixed expected status for git repository create with bare flag
idanovo Feb 24, 2022
f7bb6ea
Fixed Lakectl doctor commmand due to changes in changes on redirect r…
idanovo Feb 25, 2022
3160b53
Reformated import order
idanovo Feb 25, 2022
08582c2
Fixed expected status for branch protected add example text
idanovo Feb 27, 2022
360d462
Reverted unrelated changes to current PR
idanovo Feb 27, 2022
3513f29
Reverted changes
idanovo Feb 27, 2022
688de92
Fixed imports order and responses naming
idanovo Feb 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions cmd/lakectl/cmd/abuse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"

"github.com/spf13/cobra"

"github.com/treeverse/lakefs/pkg/api"
"github.com/treeverse/lakefs/pkg/api/helpers"
"github.com/treeverse/lakefs/pkg/testutil/stress"
Expand Down Expand Up @@ -119,7 +120,7 @@ var abuseRandomWritesCmd = &cobra.Command{

client := getClient()
resp, err := client.GetRepositoryWithResponse(cmd.Context(), u.Repository)
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion - pick a shorter name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DieOnUnexpectedResponse?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, no good suggestion


repo := resp.JSON200
storagePrefix := repo.StorageNamespace
Expand Down Expand Up @@ -173,19 +174,19 @@ var abuseCreateBranchesCmd = &cobra.Command{
currentOffset := api.PaginationAfter(branchPrefix)
amount := api.PaginationAmount(paginationAmount)
for {
res, err := client.ListBranchesWithResponse(cmd.Context(), u.Repository, &api.ListBranchesParams{
resp, err := client.ListBranchesWithResponse(cmd.Context(), u.Repository, &api.ListBranchesParams{
After: &currentOffset,
Amount: &amount,
})
DieOnResponseError(res, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)

for _, ref := range res.JSON200.Results {
for _, ref := range resp.JSON200.Results {
if !strings.HasPrefix(ref.Id, branchPrefix) {
return
}
add(ref.Id) // this branch should be deleted!
}
pagination := res.JSON200.Pagination
pagination := resp.JSON200.Pagination
if !pagination.HasMore {
return
}
Expand Down
1 change: 1 addition & 0 deletions cmd/lakectl/cmd/action_validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"io"

"github.com/spf13/cobra"

"github.com/treeverse/lakefs/pkg/actions"
"github.com/treeverse/lakefs/pkg/cmdutils"
)
Expand Down
18 changes: 10 additions & 8 deletions cmd/lakectl/cmd/annotate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package cmd

import (
"net/http"
"strings"

"github.com/spf13/cobra"

"github.com/treeverse/lakefs/pkg/api"
)

Expand Down Expand Up @@ -31,29 +33,29 @@ var annotateCmd = &cobra.Command{
client := getClient()
pfx := api.PaginationPrefix(*pathURI.Path)
context := cmd.Context()
res, err := client.ListObjectsWithResponse(context, pathURI.Repository, pathURI.Ref, &api.ListObjectsParams{Prefix: &pfx})
DieOnResponseError(res, err)
resp, err := client.ListObjectsWithResponse(context, pathURI.Repository, pathURI.Ref, &api.ListObjectsParams{Prefix: &pfx})
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)
var from string
for {
params := &api.ListObjectsParams{
Prefix: &pfx,
After: api.PaginationAfterPtr(from),
}
resp, err := client.ListObjectsWithResponse(context, pathURI.Repository, pathURI.Ref, params)
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)
for _, obj := range resp.JSON200.Results {
logCommitsParams := &api.LogCommitsParams{
Amount: api.PaginationAmountPtr(1),
Objects: &[]string{obj.Path},
}
res, err := client.LogCommitsWithResponse(context, pathURI.Repository, pathURI.Ref, logCommitsParams)
DieOnResponseError(res, err)
resp, err := client.LogCommitsWithResponse(context, pathURI.Repository, pathURI.Ref, logCommitsParams)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is actually incorrect, because l.51 assigned to res. Response variables are clearly a mess in here. Right now you're the victim, and the only way out might be to fix them entirely.

If you can, I would feel safely if you could: pick one spelling, and search-and-destroy the entire cmd directory for the other one (git grep -w res cmd/ is your friend, and should return no results once we're done.)

Copy link
Contributor

Choose a reason for hiding this comment

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

tag.go still contains a variable res:

[ariels@fedora lakeFS]$ git grep -w res cmd/
cmd/lakectl/cmd/tag.go:                 res, err := client.GetCommitWithResponse(ctx, tagURI.Repository, commitRef)
cmd/lakectl/cmd/tag.go:                 DieOnErrorOrUnexpectedStatusCode(res, err, http.StatusOK)

This inconsistency is precisely what got us into trouble on this line. So could you fix that one too, please, even though it is not strictly related to this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

My 2 cents - use different names so we will not verify or check the wrong response by mistake

data := objectCommitData{
Object: obj.Path,
}
if len(res.JSON200.Results) > 0 {
data.Commit = res.JSON200.Results[0]
data.CommitMessage = splitOnNewLine(stringTrimLen((res.JSON200.Results[0].Message), annotateMessageSize))
if len(resp.JSON200.Results) > 0 {
data.Commit = resp.JSON200.Results[0]
data.CommitMessage = splitOnNewLine(stringTrimLen((resp.JSON200.Results[0].Message), annotateMessageSize))
}
Write(annotateTemplate, data)
}
Expand Down
54 changes: 28 additions & 26 deletions cmd/lakectl/cmd/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ package cmd
import (
"encoding/json"
"io"
"net/http"
"os"
"strings"
"time"

"github.com/spf13/cobra"

"github.com/treeverse/lakefs/pkg/api"
)

Expand Down Expand Up @@ -63,7 +65,7 @@ var authUsersList = &cobra.Command{
After: api.PaginationAfterPtr(after),
Amount: api.PaginationAmountPtr(amount),
})
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)

users := resp.JSON200.Results
rows := make([][]interface{}, len(users))
Expand All @@ -87,7 +89,7 @@ var authUsersCreate = &cobra.Command{
resp, err := clt.CreateUserWithResponse(cmd.Context(), api.CreateUserJSONRequestBody{
Id: id,
})
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusCreated)
user := resp.JSON201
Write(userCreatedTemplate, user)
},
Expand All @@ -101,7 +103,7 @@ var authUsersDelete = &cobra.Command{
clt := getClient()

resp, err := clt.DeleteUserWithResponse(cmd.Context(), id)
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, defaultResponseOnSwaggerClient)
Fmt("User deleted successfully\n")
},
}
Expand All @@ -125,7 +127,7 @@ var authUsersGroupsList = &cobra.Command{
After: api.PaginationAfterPtr(after),
Amount: api.PaginationAmountPtr(amount),
})
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)

groups := resp.JSON200.Results
rows := make([][]interface{}, len(groups))
Expand Down Expand Up @@ -160,7 +162,7 @@ var authUsersPoliciesList = &cobra.Command{
Amount: api.PaginationAmountPtr(amount),
Effective: &effective,
})
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)

policies := resp.JSON200.Results
rows := make([][]interface{}, 0)
Expand All @@ -185,7 +187,7 @@ var authUsersPoliciesAttach = &cobra.Command{
policy, _ := cmd.Flags().GetString("policy")
clt := getClient()
resp, err := clt.AttachPolicyToUserWithResponse(cmd.Context(), id, policy)
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusCreated)
Fmt("Policy attached successfully\n")
},
}
Expand All @@ -199,7 +201,7 @@ var authUsersPoliciesDetach = &cobra.Command{
clt := getClient()

resp, err := clt.DetachPolicyFromUserWithResponse(cmd.Context(), id, policy)
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, defaultResponseOnSwaggerClient)

Fmt("Policy detached successfully\n")
},
Expand All @@ -219,12 +221,12 @@ var authUsersCredentialsCreate = &cobra.Command{

if id == "" {
resp, err := clt.GetCurrentUserWithResponse(cmd.Context())
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)
id = resp.JSON200.User.Id
}

resp, err := clt.CreateCredentialsWithResponse(cmd.Context(), id)
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusCreated)

credentials := resp.JSON201
Write(credentialsCreatedTemplate, credentials)
Expand All @@ -241,11 +243,11 @@ var authUsersCredentialsDelete = &cobra.Command{

if id == "" {
resp, err := clt.GetCurrentUserWithResponse(cmd.Context())
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)
id = resp.JSON200.User.Id
}
resp, err := clt.DeleteCredentialsWithResponse(cmd.Context(), id, accessKeyID)
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, defaultResponseOnSwaggerClient)

Fmt("Credentials deleted successfully\n")
},
Expand All @@ -262,15 +264,15 @@ var authUsersCredentialsList = &cobra.Command{
clt := getClient()
if id == "" {
resp, err := clt.GetCurrentUserWithResponse(cmd.Context())
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)
id = resp.JSON200.User.Id
}

resp, err := clt.ListUserCredentialsWithResponse(cmd.Context(), id, &api.ListUserCredentialsParams{
After: api.PaginationAfterPtr(after),
Amount: api.PaginationAmountPtr(amount),
})
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)

credentials := resp.JSON200.Results
rows := make([][]interface{}, len(credentials))
Expand Down Expand Up @@ -302,7 +304,7 @@ var authGroupsList = &cobra.Command{
After: api.PaginationAfterPtr(after),
Amount: api.PaginationAmountPtr(amount),
})
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)

groups := resp.JSON200.Results
rows := make([][]interface{}, len(groups))
Expand All @@ -326,7 +328,7 @@ var authGroupsCreate = &cobra.Command{
resp, err := clt.CreateGroupWithResponse(cmd.Context(), api.CreateGroupJSONRequestBody{
Id: id,
})
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusCreated)
group := resp.JSON201
Write(groupCreatedTemplate, group)
},
Expand All @@ -340,7 +342,7 @@ var authGroupsDelete = &cobra.Command{
clt := getClient()

resp, err := clt.DeleteGroupWithResponse(cmd.Context(), id)
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, defaultResponseOnSwaggerClient)
Fmt("Group deleted successfully\n")
},
}
Expand All @@ -364,7 +366,7 @@ var authGroupsListMembers = &cobra.Command{
After: api.PaginationAfterPtr(after),
Amount: api.PaginationAmountPtr(amount),
})
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)

users := resp.JSON200.Results
rows := make([][]interface{}, len(users))
Expand All @@ -386,7 +388,7 @@ var authGroupsAddMember = &cobra.Command{
clt := getClient()

resp, err := clt.AddGroupMembershipWithResponse(cmd.Context(), id, user)
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusCreated)
Fmt("User successfully added\n")
},
}
Expand All @@ -400,7 +402,7 @@ var authGroupsRemoveMember = &cobra.Command{
clt := getClient()

resp, err := clt.DeleteGroupMembershipWithResponse(cmd.Context(), id, user)
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, defaultResponseOnSwaggerClient)
Fmt("User successfully removed\n")
},
}
Expand All @@ -424,7 +426,7 @@ var authGroupsPoliciesList = &cobra.Command{
After: api.PaginationAfterPtr(after),
Amount: api.PaginationAmountPtr(amount),
})
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)

policies := resp.JSON200.Results
rows := make([][]interface{}, 0)
Expand All @@ -450,7 +452,7 @@ var authGroupsPoliciesAttach = &cobra.Command{
clt := getClient()

resp, err := clt.AttachPolicyToGroupWithResponse(cmd.Context(), id, policy)
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusCreated)

Fmt("Policy attached successfully\n")
},
Expand All @@ -465,7 +467,7 @@ var authGroupsPoliciesDetach = &cobra.Command{
clt := getClient()

resp, err := clt.DetachPolicyFromGroupWithResponse(cmd.Context(), id, policy)
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, defaultResponseOnSwaggerClient)

Fmt("Policy detached successfully\n")
},
Expand All @@ -490,7 +492,7 @@ var authPoliciesList = &cobra.Command{
After: api.PaginationAfterPtr(after),
Amount: api.PaginationAmountPtr(amount),
})
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)

policies := resp.JSON200.Results
rows := make([][]interface{}, len(policies))
Expand Down Expand Up @@ -534,7 +536,7 @@ var authPoliciesCreate = &cobra.Command{
Id: id,
Statement: doc.Statement,
})
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusCreated)

createdPolicy := resp.JSON201
Write(policyCreatedTemplate, struct {
Expand All @@ -561,7 +563,7 @@ var authPoliciesShow = &cobra.Command{
clt := getClient()

resp, err := clt.GetPolicyWithResponse(cmd.Context(), id)
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusOK)

policy := *resp.JSON200
Write(policyDetailsTemplate, struct {
Expand All @@ -584,7 +586,7 @@ var authPoliciesDelete = &cobra.Command{
clt := getClient()

resp, err := clt.DeletePolicyWithResponse(cmd.Context(), id)
DieOnResponseError(resp, err)
DieOnErrorOrUnexpectedStatusCode(resp, err, defaultResponseOnSwaggerClient)
Fmt("Policy deleted successfully\n")
},
}
Expand Down
Loading