Skip to content

Commit

Permalink
Update cluster cmd (#1487)
Browse files Browse the repository at this point in the history
* Remove auth flow from cluster add/delete

* fix lint error

* remove redundant code

Co-authored-by: Justin Thompson <jpthomp12@gmail.com>
  • Loading branch information
J-Thompson12 and Justin Thompson committed Feb 24, 2022
1 parent 6baf29d commit 918ba04
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 98 deletions.
3 changes: 1 addition & 2 deletions cmd/gitops/add/clusters/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/weaveworks/weave-gitops/pkg/adapters"
"github.com/weaveworks/weave-gitops/pkg/capi"
"github.com/weaveworks/weave-gitops/pkg/gitproviders"
"github.com/weaveworks/weave-gitops/pkg/services/auth"
)

type clusterCommandFlags struct {
Expand Down Expand Up @@ -122,7 +121,7 @@ func getClusterCmdRunE(endpoint *string, client *resty.Client) func(*cobra.Comma
return fmt.Errorf("cannot parse url: %w", err)
}

token, err := internal.GetToken(url, os.Stdout, os.LookupEnv, auth.NewAuthCLIHandler, internal.NewCLILogger(os.Stdout))
token, err := internal.GetToken(url, os.LookupEnv)
if err != nil {
return err
}
Expand Down
3 changes: 1 addition & 2 deletions cmd/gitops/delete/clusters/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/weaveworks/weave-gitops/pkg/adapters"
"github.com/weaveworks/weave-gitops/pkg/clusters"
"github.com/weaveworks/weave-gitops/pkg/gitproviders"
"github.com/weaveworks/weave-gitops/pkg/services/auth"
)

type clustersDeleteFlags struct {
Expand Down Expand Up @@ -78,7 +77,7 @@ func getClusterCmdRunE(endpoint *string, client *resty.Client) func(*cobra.Comma
return fmt.Errorf("cannot parse url: %w", err)
}

token, err := internal.GetToken(url, os.Stdout, os.LookupEnv, auth.NewAuthCLIHandler, internal.NewCLILogger(os.Stdout))
token, err := internal.GetToken(url, os.LookupEnv)
if err != nil {
return err
}
Expand Down
32 changes: 6 additions & 26 deletions cmd/internal/provider.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
package internal

import (
"context"
"fmt"
"io"
"os"

"github.com/weaveworks/weave-gitops/pkg/gitproviders"
"github.com/weaveworks/weave-gitops/pkg/logger"
)

const envVariableWarning = "Setting the %q environment variable to a valid token will allow ongoing use of the CLI without requiring a browser-based auth flow...\n"
const missingTokenErr = "the %q environment variable needs to be set to a valid token"

type gitProviderClient struct {
authHandlerFunc GetAuthHandler
Expand All @@ -28,10 +26,9 @@ func NewGitProviderClient(stdout *os.File, lookupEnvFunc func(key string) (strin
}
}

// GetProvider returns a GitProvider containing either the token stored in the <git provider>_TOKEN env var
// or a token retrieved via the CLI auth flow
// GetProvider returns a GitProvider containing the token stored in the <git provider>_TOKEN
func (c *gitProviderClient) GetProvider(repoUrl gitproviders.RepoURL, getAccountType gitproviders.AccountTypeGetter) (gitproviders.GitProvider, error) {
token, err := GetToken(repoUrl, c.stdout, c.lookupEnvFunc, c.authHandlerFunc, c.log)
token, err := GetToken(repoUrl, c.lookupEnvFunc)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -59,33 +56,16 @@ func getTokenVarName(providerName gitproviders.GitProviderName) (string, error)
}
}

// GetToken returns either the token stored in the <git provider>_TOKEN env var
// or a token retrieved via the CLI auth flow
func GetToken(repoUrl gitproviders.RepoURL, w io.Writer, lookupEnvFunc func(key string) (string, bool), authHandlerFunc GetAuthHandler, log logger.Logger) (string, error) {
// GetToken returns the token stored in the <git provider>_TOKEN env var
func GetToken(repoUrl gitproviders.RepoURL, lookupEnvFunc func(key string) (string, bool)) (string, error) {
tokenVarName, err := getTokenVarName(repoUrl.Provider())
if err != nil {
return "", fmt.Errorf("could not determine git provider token name: %w", err)
}

token, exists := lookupEnvFunc(tokenVarName)
if !exists {
log.Warningf(envVariableWarning, tokenVarName)

authHandler, err := authHandlerFunc(repoUrl.Provider())
if err != nil {
return "", fmt.Errorf("error initializing cli auth handler: %w", err)
}

ctx := context.Background()

generatedToken, err := authHandler(ctx, w)
if err != nil {
return "", fmt.Errorf("could not complete auth flow: %w", err)
}

token = generatedToken
} else if err != nil {
return "", fmt.Errorf("could not get access token: %w", err)
return "", fmt.Errorf(missingTokenErr, tokenVarName)
}

return token, nil
Expand Down
68 changes: 0 additions & 68 deletions cmd/internal/provider_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
package internal

import (
"bytes"
"context"
"errors"
"fmt"
"io"
"os"

"github.com/fluxcd/go-git-providers/gitprovider"
Expand All @@ -21,22 +18,6 @@ const (
gitlabToken = "gitlab-token-abc"
)

func fakeBlockingCLIHandlerSuccess(_ context.Context, _ io.Writer) (string, error) {
return githubToken, nil
}

func fakeBlockingCLIHandlerError(_ context.Context, _ io.Writer) (string, error) {
return "", errors.New("blocking cli handler goes ka-boom")
}

func fakeAuthHandlerFuncGoodCLI(_ gitproviders.GitProviderName) (auth.BlockingCLIAuthHandler, error) {
return fakeBlockingCLIHandlerSuccess, nil
}

func fakeAuthHandlerFuncBadCLI(_ gitproviders.GitProviderName) (auth.BlockingCLIAuthHandler, error) {
return fakeBlockingCLIHandlerError, nil
}

func fakeAuthHandlerFuncError(_ gitproviders.GitProviderName) (auth.BlockingCLIAuthHandler, error) {
return nil, errors.New("get auth handler goes ka-boom")
}
Expand All @@ -59,10 +40,6 @@ func fakeEnvLookupExists(key string) (string, bool) {
}
}

func fakeEnvLookupDoesNotExist(key string) (string, bool) {
return "", false
}

var _ = Describe("Get git provider", func() {
var client gitproviders.Client
var repoUrl gitproviders.RepoURL
Expand Down Expand Up @@ -129,49 +106,4 @@ var _ = Describe("Get git provider", func() {
})
})
})

Describe("auth flow since token is not in an env variable", func() {
BeforeEach(func() {
fakeLogger = &loggerfakes.FakeLogger{
WarningfStub: func(fmtArg string, restArgs ...interface{}) {},
}
repoUrl, _ = gitproviders.NewRepoURL("ssh://git@github.com/weaveworks/weave-gitops.git")
})

AfterEach(func() {
fmtArg, restArgs := fakeLogger.WarningfArgsForCall(0)
Expect(fmtArg).Should(Equal(envVariableWarning))
Expect(restArgs).To(HaveLen(1))
Expect(restArgs[0]).Should(Equal("GITHUB_TOKEN"))
Expect(fakeLogger.WarningfCallCount()).To(Equal(1))
})

It("cannot generate auth flow handler", func() {
client = NewGitProviderClient(os.Stdout, fakeEnvLookupDoesNotExist, fakeAuthHandlerFuncError, fakeLogger)
provider, err := client.GetProvider(repoUrl, fakeAccountGetterError)

Expect(provider).To(BeNil())
_, expectedErr := fakeAuthHandlerFuncError(repoUrl.Provider())
Expect(err).To(MatchError(fmt.Errorf("error initializing cli auth handler: %w", expectedErr)))
})

It("blocking cli handler returns an error during auth flow", func() {
client = NewGitProviderClient(os.Stdout, fakeEnvLookupDoesNotExist, fakeAuthHandlerFuncBadCLI, fakeLogger)
provider, err := client.GetProvider(repoUrl, fakeAccountGetterError)

Expect(provider).To(BeNil())
_, expectedErr := fakeBlockingCLIHandlerError(context.Background(), bytes.NewBufferString(""))
Expect(err).To(MatchError(fmt.Errorf("could not complete auth flow: %w", expectedErr)))
})

It("success", func() {
client = NewGitProviderClient(os.Stdout, fakeEnvLookupDoesNotExist, fakeAuthHandlerFuncGoodCLI, fakeLogger)
provider, err := client.GetProvider(repoUrl, fakeAccountGetterSuccess)

Expect(err).To(BeNil())
expectedProvider, _ := gitproviders.New(gitproviders.Config{Provider: repoUrl.Provider(), Token: githubToken}, repoUrl.Owner(), fakeAccountGetterSuccess)
Expect(provider).To(Equal(expectedProvider))
})
})

})

0 comments on commit 918ba04

Please sign in to comment.