feat: Accept repository token for remote config OD-497 #206
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,68 @@ | ||
| package cmdutils | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "log" | ||
| "os" | ||
|
|
||
| "codacy/cli-v2/domain" | ||
|
|
||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| // ProjectTokenEnvVar is also the codacy-coverage-reporter's variable, so it is often | ||
| // exported for unrelated reasons - see ResolveProjectToken. | ||
| const ProjectTokenEnvVar = "CODACY_PROJECT_TOKEN" | ||
|
|
||
| // AddCloudFlags adds the common cloud-related flags to a cobra command. | ||
| // The flags will be bound to the provided flags struct. | ||
| func AddCloudFlags(cmd *cobra.Command, flags *domain.InitFlags) { | ||
| cmd.Flags().StringVar(&flags.ApiToken, "api-token", "", "Optional Codacy account API token. If defined, configurations will be fetched from Codacy") | ||
|
There was a problem hiding this comment. 🟡 MEDIUM RISK Suggestion: The flag registration logic in |
||
| cmd.Flags().StringVar(&flags.ProjectToken, "project-token", "", "Optional Codacy repository token, an alternative to api-token for fetching configurations from Codacy. Falls back to CODACY_PROJECT_TOKEN when provider, organization and repository are given. See https://docs.codacy.com/codacy-api/api-tokens/#repository-api-tokens") | ||
| cmd.Flags().StringVar(&flags.Provider, "provider", "", "Provider (e.g., gh, bb, gl) to fetch configurations from Codacy. Required when api-token or project-token is provided") | ||
| cmd.Flags().StringVar(&flags.Organization, "organization", "", "Remote organization name to fetch configurations from Codacy. Required when api-token or project-token is provided") | ||
| cmd.Flags().StringVar(&flags.Repository, "repository", "", "Remote repository name to fetch configurations from Codacy. Required when api-token or project-token is provided") | ||
| } | ||
|
|
||
| // ResolveProjectToken falls back to CODACY_PROJECT_TOKEN, but only once the repository | ||
| // coordinates are known. The variable is the coverage reporter's and is routinely exported | ||
| // CI-wide, so on its own it must not switch a local run into remote mode. | ||
| func ResolveProjectToken(flags *domain.InitFlags) { | ||
| if flags.HasRemoteToken() || !flags.HasRepositoryCoordinates() { | ||
| return | ||
| } | ||
| flags.ProjectToken = os.Getenv(ProjectTokenEnvVar) | ||
| } | ||
|
|
||
| // ValidateRemoteFlags rejects a token given without the repository coordinates every | ||
| // remote read needs, so commands fail with a clear message instead of requesting a URL | ||
| // with empty path segments. | ||
| func ValidateRemoteFlags(flags domain.InitFlags) error { | ||
| if !flags.HasRemoteToken() || flags.HasRepositoryCoordinates() { | ||
| return nil | ||
| } | ||
| return errors.New("when using --api-token or --project-token, you must also provide --provider, --organization, and --repository flags") | ||
| } | ||
|
|
||
| // exit is swappable so the failure path stays testable. | ||
| var exit = os.Exit | ||
|
|
||
| // PrepareRemoteFlags resolves the environment fallback and rejects an incomplete remote | ||
| // setup, so every command that takes cloud flags behaves the same way. | ||
| func PrepareRemoteFlags(cmd *cobra.Command, flags *domain.InitFlags) { | ||
| ResolveProjectToken(flags) | ||
|
|
||
| err := ValidateRemoteFlags(*flags) | ||
| if err == nil { | ||
| return | ||
| } | ||
|
|
||
| fmt.Printf("Error: %s.\n", err) | ||
| fmt.Println("Please provide all required flags and try again.") | ||
| fmt.Println() | ||
| if errHelp := cmd.Help(); errHelp != nil { | ||
| log.Printf("Warning: Failed to display command help: %v", errHelp) | ||
| } | ||
| exit(1) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| package cmdutils | ||
|
|
||
| import ( | ||
| "io" | ||
| "testing" | ||
|
|
||
| "codacy/cli-v2/domain" | ||
|
|
||
| "github.com/spf13/cobra" | ||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func completeFlags() domain.InitFlags { | ||
| return domain.InitFlags{Provider: "gh", Organization: "org", Repository: "repo"} | ||
| } | ||
|
|
||
| func TestAddCloudFlags(t *testing.T) { | ||
| t.Setenv(ProjectTokenEnvVar, "token-from-env") | ||
|
|
||
| cmd := &cobra.Command{Use: "test"} | ||
| flags := domain.InitFlags{} | ||
| AddCloudFlags(cmd, &flags) | ||
|
|
||
| for _, name := range []string{"api-token", "project-token", "provider", "organization", "repository"} { | ||
| assert.NotNil(t, cmd.Flags().Lookup(name), "flag %s should be registered", name) | ||
| } | ||
|
|
||
| // the environment must not leak in at registration time - see ResolveProjectToken | ||
| assert.Equal(t, "", cmd.Flags().Lookup("project-token").DefValue) | ||
| assert.Equal(t, domain.InitFlags{}, flags) | ||
| } | ||
|
|
||
| func TestAddCloudFlagsBindsValues(t *testing.T) { | ||
| cmd := &cobra.Command{Use: "test", Run: func(*cobra.Command, []string) {}} | ||
| flags := domain.InitFlags{} | ||
| AddCloudFlags(cmd, &flags) | ||
|
|
||
| cmd.SetArgs([]string{"--project-token", "tok", "--provider", "gh", "--organization", "org", "--repository", "repo"}) | ||
| assert.NoError(t, cmd.Execute()) | ||
|
|
||
| assert.Equal(t, domain.InitFlags{ProjectToken: "tok", Provider: "gh", Organization: "org", Repository: "repo"}, flags) | ||
| assert.NoError(t, ValidateRemoteFlags(flags)) | ||
| } | ||
|
|
||
| func TestResolveProjectToken(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| env string | ||
| flags domain.InitFlags | ||
| expectedToken string | ||
| }{ | ||
| {"environment applies once coordinates are known", "env-token", completeFlags(), "env-token"}, | ||
| {"environment ignored without coordinates", "env-token", domain.InitFlags{}, ""}, | ||
| {"environment ignored with partial coordinates", "env-token", domain.InitFlags{Provider: "gh", Organization: "org"}, ""}, | ||
| {"explicit flag wins over environment", "env-token", domain.InitFlags{ProjectToken: "flag-token", Provider: "gh", Organization: "org", Repository: "repo"}, "flag-token"}, | ||
| {"account token is not overwritten", "env-token", domain.InitFlags{ApiToken: "api", Provider: "gh", Organization: "org", Repository: "repo"}, ""}, | ||
| {"unset environment leaves flags alone", "", completeFlags(), ""}, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Setenv(ProjectTokenEnvVar, tt.env) | ||
|
|
||
| flags := tt.flags | ||
| ResolveProjectToken(&flags) | ||
|
|
||
| assert.Equal(t, tt.expectedToken, flags.ProjectToken) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestValidateRemoteFlags(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| flags domain.InitFlags | ||
| expectError bool | ||
| }{ | ||
| {"no token needs no coordinates", domain.InitFlags{}, false}, | ||
| {"no token with partial coordinates", domain.InitFlags{Provider: "gh"}, false}, | ||
| {"account token with coordinates", domain.InitFlags{ApiToken: "api", Provider: "gh", Organization: "org", Repository: "repo"}, false}, | ||
| {"repository token with coordinates", domain.InitFlags{ProjectToken: "project", Provider: "gh", Organization: "org", Repository: "repo"}, false}, | ||
| {"repository token without coordinates", domain.InitFlags{ProjectToken: "project"}, true}, | ||
| {"account token without coordinates", domain.InitFlags{ApiToken: "api"}, true}, | ||
| {"token missing provider", domain.InitFlags{ProjectToken: "project", Organization: "org", Repository: "repo"}, true}, | ||
| {"token missing organization", domain.InitFlags{ProjectToken: "project", Provider: "gh", Repository: "repo"}, true}, | ||
| {"token missing repository", domain.InitFlags{ProjectToken: "project", Provider: "gh", Organization: "org"}, true}, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| err := ValidateRemoteFlags(tt.flags) | ||
| if tt.expectError { | ||
| assert.Error(t, err) | ||
| } else { | ||
| assert.NoError(t, err) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // stopped reports whether PrepareRemoteFlags terminated the command. The exit stub panics | ||
| // so a helper that kept running past it would fail the assertion instead of passing. | ||
| func stopped(t *testing.T, flags *domain.InitFlags) bool { | ||
| t.Helper() | ||
|
|
||
| original := exit | ||
| defer func() { exit = original }() | ||
|
|
||
| type exitCall struct{ code int } | ||
| exit = func(code int) { panic(exitCall{code}) } | ||
|
|
||
| cmd := &cobra.Command{Use: "test"} | ||
| cmd.SetOut(io.Discard) | ||
|
|
||
| didExit := false | ||
| func() { | ||
| defer func() { | ||
| if r := recover(); r != nil { | ||
| call, ok := r.(exitCall) | ||
| assert.True(t, ok, "unexpected panic: %v", r) | ||
| assert.Equal(t, 1, call.code) | ||
| didExit = true | ||
| } | ||
| }() | ||
| PrepareRemoteFlags(cmd, flags) | ||
| }() | ||
|
|
||
| return didExit | ||
| } | ||
|
|
||
| func TestPrepareRemoteFlags(t *testing.T) { | ||
| t.Run("stops on a token without coordinates", func(t *testing.T) { | ||
| flags := domain.InitFlags{ProjectToken: "tok"} | ||
| assert.True(t, stopped(t, &flags)) | ||
| }) | ||
|
|
||
| t.Run("continues without any token", func(t *testing.T) { | ||
| flags := domain.InitFlags{} | ||
| assert.False(t, stopped(t, &flags)) | ||
| }) | ||
|
|
||
| t.Run("continues when the environment token is ignored", func(t *testing.T) { | ||
| t.Setenv(ProjectTokenEnvVar, "env-token") | ||
|
|
||
| flags := domain.InitFlags{} | ||
| assert.False(t, stopped(t, &flags)) | ||
| assert.Equal(t, "", flags.ProjectToken) | ||
| }) | ||
|
|
||
| t.Run("adopts the environment token with coordinates", func(t *testing.T) { | ||
| t.Setenv(ProjectTokenEnvVar, "env-token") | ||
|
|
||
| flags := completeFlags() | ||
| assert.False(t, stopped(t, &flags)) | ||
| assert.Equal(t, "env-token", flags.ProjectToken) | ||
| }) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Suggestion: This condition
(!cliLocalMode && initFlags.HasRemoteToken()) || cliLocalModealways evaluates totruebecausecliLocalModeis the negation ofHasRemoteToken(). This makes theelseblock on line 350 unreachable dead code, including the log message on line 354. The logic should be simplified or corrected to ensure the intended fallback behavior works.