Skip to content

Commit

Permalink
added golangci-lint (#168)
Browse files Browse the repository at this point in the history
* * added golangci-lint.yml and fixed linter issues
* bumped to 1.19

* updated golangci version
  • Loading branch information
weinong authored Dec 8, 2022
1 parent 77d57a5 commit 992e74d
Show file tree
Hide file tree
Showing 14 changed files with 128 additions and 85 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ jobs:
env:
GO111MODULE: on
steps:
- name: Set up Go 1.18
- name: Set up Go 1.19
uses: actions/setup-go@v3
with:
go-version: 1.18
go-version: 1.19
id: go

- name: Check out code into the Go module directory
Expand Down
27 changes: 27 additions & 0 deletions .github/workflows/golangci-lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
name: golangci-lint
on:
push:
tags:
- v*
branches:
- master
- main
pull_request:
permissions:
contents: read
# Optional: allow read access to pull request. Use with `only-new-issues` option.
# pull-requests: read
jobs:
golangci:
name: lint
runs-on: ubuntu-latest
steps:
- uses: actions/setup-go@v3
with:
go-version: 1.19
- uses: actions/checkout@v3
- name: golangci-lint
uses: golangci/golangci-lint-action@v3
with:
# Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
version: v1.50
4 changes: 2 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ jobs:
env:
GO111MODULE: on
steps:
- name: Set up Go 1.18
- name: Set up Go 1.19
uses: actions/setup-go@v3
with:
go-version: 1.18
go-version: 1.19
id: go

- name: Check out code into the Go module directory
Expand Down
33 changes: 33 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
run:
# default concurrency is a available CPU number
concurrency: 4
deadline: 10m
tests: false

linters:
disable-all: true
enable:
- errcheck
- goconst
- gocritic
- goimports
- gosec
- govet
- ineffassign
- misspell
- paralleltest
- staticcheck
- stylecheck
- unused

linters-settings:
gocritic:
disabled-checks:
- ifElseChain
misspell:
locale: US
gosec:
excludes:
- G101
goimports:
local-prefixes: github.com/org/project
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ LDFLAGS := -X main.version=$(VERSION) \

all: $(TARGET)

test:
lint:
hack/verify-golangci-lint.sh

test: lint
go test -race -coverprofile=coverage.txt -covermode=atomic ./...

version:
Expand Down
11 changes: 11 additions & 0 deletions hack/verify-golangci-lint.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/bash -e

# this script is invoked by the Makefile and is not used in the pipeline as it's done by a github actiono
# so this script is intended for local testing
if ! command -v golangci-lint &> /dev/null
then
echo "WARNING: golangci-lint is not found. Hence linting is skipped. Visit https://golangci-lint.run/usage/install/#local-installation to install"
else
golangci-lint run
fi

2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ func main() {
klog.InitFlags(nil)
pflag.CommandLine.AddGoFlag(flag.CommandLine.Lookup("v"))
pflag.CommandLine.AddGoFlag(flag.CommandLine.Lookup("logtostderr"))
pflag.CommandLine.Set("logtostderr", "true")
_ = pflag.CommandLine.Set("logtostderr", "true")
root := cmd.NewRootCmd(v.String())
if err := root.Execute(); err != nil {
os.Exit(1)
Expand Down
81 changes: 27 additions & 54 deletions pkg/converter/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,19 +164,16 @@ func Convert(o Options, pathOptions *clientcmd.PathOptions) error {
APIVersion: execAPIVersion,
}

exec.Args = append(exec.Args, argLoginMethod)
exec.Args = append(exec.Args, o.TokenOptions.LoginMethod)
exec.Args = append(exec.Args, argLoginMethod, o.TokenOptions.LoginMethod)

// all login methods require --server-id specified
if argServerIDVal == "" {
return fmt.Errorf("%s is required", argServerID)
}
exec.Args = append(exec.Args, argServerID)
exec.Args = append(exec.Args, argServerIDVal)
exec.Args = append(exec.Args, argServerID, argServerIDVal)

if argTokenCacheDirVal != "" {
exec.Args = append(exec.Args, argTokenCacheDir)
exec.Args = append(exec.Args, argTokenCacheDirVal)
exec.Args = append(exec.Args, argTokenCacheDir, argTokenCacheDirVal)
}

switch o.TokenOptions.LoginMethod {
Expand All @@ -187,8 +184,7 @@ func Convert(o Options, pathOptions *clientcmd.PathOptions) error {
// this is because azure cli logged in using MSI does not allow specifying tenant ID
// see https://github.com/Azure/kubelogin/issues/123#issuecomment-1209652342
if o.isSet(flagTenantID) {
exec.Args = append(exec.Args, argTenantID)
exec.Args = append(exec.Args, o.TokenOptions.TenantID)
exec.Args = append(exec.Args, argTenantID, o.TokenOptions.TenantID)
}

case token.DeviceCodeLogin:
Expand All @@ -197,20 +193,17 @@ func Convert(o Options, pathOptions *clientcmd.PathOptions) error {
return fmt.Errorf("%s is required", argClientID)
}

exec.Args = append(exec.Args, argClientID)
exec.Args = append(exec.Args, argClientIDVal)
exec.Args = append(exec.Args, argClientID, argClientIDVal)

if argTenantIDVal == "" {
return fmt.Errorf("%s is required", argTenantID)
}

exec.Args = append(exec.Args, argTenantID)
exec.Args = append(exec.Args, argTenantIDVal)
exec.Args = append(exec.Args, argTenantID, argTenantIDVal)

if argEnvironmentVal != "" {
// environment is optional
exec.Args = append(exec.Args, argEnvironment)
exec.Args = append(exec.Args, argEnvironmentVal)
exec.Args = append(exec.Args, argEnvironment, argEnvironmentVal)
}

if isLegacyConfigMode {
Expand All @@ -223,20 +216,17 @@ func Convert(o Options, pathOptions *clientcmd.PathOptions) error {
return fmt.Errorf("%s is required", argClientID)
}

exec.Args = append(exec.Args, argClientID)
exec.Args = append(exec.Args, argClientIDVal)
exec.Args = append(exec.Args, argClientID, argClientIDVal)

if argTenantIDVal == "" {
return fmt.Errorf("%s is required", argTenantID)
}

exec.Args = append(exec.Args, argTenantID)
exec.Args = append(exec.Args, argTenantIDVal)
exec.Args = append(exec.Args, argTenantID, argTenantIDVal)

if argEnvironmentVal != "" {
// environment is optional
exec.Args = append(exec.Args, argEnvironment)
exec.Args = append(exec.Args, argEnvironmentVal)
exec.Args = append(exec.Args, argEnvironment, argEnvironmentVal)
}

case token.ServicePrincipalLogin:
Expand All @@ -245,35 +235,29 @@ func Convert(o Options, pathOptions *clientcmd.PathOptions) error {
return fmt.Errorf("%s is required", argClientID)
}

exec.Args = append(exec.Args, argClientID)
exec.Args = append(exec.Args, argClientIDVal)
exec.Args = append(exec.Args, argClientID, argClientIDVal)

if argTenantIDVal == "" {
return fmt.Errorf("%s is required", argTenantID)
}

exec.Args = append(exec.Args, argTenantID)
exec.Args = append(exec.Args, argTenantIDVal)
exec.Args = append(exec.Args, argTenantID, argTenantIDVal)

if argEnvironmentVal != "" {
// environment is optional
exec.Args = append(exec.Args, argEnvironment)
exec.Args = append(exec.Args, argEnvironmentVal)
exec.Args = append(exec.Args, argEnvironment, argEnvironmentVal)
}

if o.isSet(flagClientSecret) {
exec.Args = append(exec.Args, argClientSecret)
exec.Args = append(exec.Args, o.TokenOptions.ClientSecret)
exec.Args = append(exec.Args, argClientSecret, o.TokenOptions.ClientSecret)
}

if o.isSet(flagClientCert) {
exec.Args = append(exec.Args, argClientCert)
exec.Args = append(exec.Args, o.TokenOptions.ClientCert)
exec.Args = append(exec.Args, argClientCert, o.TokenOptions.ClientCert)
}

if o.isSet(flagClientCertPassword) {
exec.Args = append(exec.Args, argClientCertPassword)
exec.Args = append(exec.Args, o.TokenOptions.ClientCertPassword)
exec.Args = append(exec.Args, argClientCertPassword, o.TokenOptions.ClientCertPassword)
}

if isLegacyConfigMode {
Expand All @@ -283,11 +267,9 @@ func Convert(o Options, pathOptions *clientcmd.PathOptions) error {
case token.MSILogin:

if o.isSet(flagClientID) {
exec.Args = append(exec.Args, argClientID)
exec.Args = append(exec.Args, o.TokenOptions.ClientID)
exec.Args = append(exec.Args, argClientID, o.TokenOptions.ClientID)
} else if o.isSet(flagIdentityResourceID) {
exec.Args = append(exec.Args, argIdentityResourceID)
exec.Args = append(exec.Args, o.TokenOptions.IdentityResourceId)
exec.Args = append(exec.Args, argIdentityResourceID, o.TokenOptions.IdentityResourceID)
}

case token.ROPCLogin:
Expand All @@ -296,30 +278,25 @@ func Convert(o Options, pathOptions *clientcmd.PathOptions) error {
return fmt.Errorf("%s is required", argClientID)
}

exec.Args = append(exec.Args, argClientID)
exec.Args = append(exec.Args, argClientIDVal)
exec.Args = append(exec.Args, argClientID, argClientIDVal)

if argTenantIDVal == "" {
return fmt.Errorf("%s is required", argTenantID)
}

exec.Args = append(exec.Args, argTenantID)
exec.Args = append(exec.Args, argTenantIDVal)
exec.Args = append(exec.Args, argTenantID, argTenantIDVal)

if argEnvironmentVal != "" {
// environment is optional
exec.Args = append(exec.Args, argEnvironment)
exec.Args = append(exec.Args, argEnvironmentVal)
exec.Args = append(exec.Args, argEnvironment, argEnvironmentVal)
}

if o.isSet(flagUsername) {
exec.Args = append(exec.Args, argUsername)
exec.Args = append(exec.Args, o.TokenOptions.Username)
exec.Args = append(exec.Args, argUsername, o.TokenOptions.Username)
}

if o.isSet(flagPassword) {
exec.Args = append(exec.Args, argPassword)
exec.Args = append(exec.Args, o.TokenOptions.Password)
exec.Args = append(exec.Args, argPassword, o.TokenOptions.Password)
}

if isLegacyConfigMode {
Expand All @@ -329,23 +306,19 @@ func Convert(o Options, pathOptions *clientcmd.PathOptions) error {
case token.WorkloadIdentityLogin:

if o.isSet(flagClientID) {
exec.Args = append(exec.Args, argClientID)
exec.Args = append(exec.Args, o.TokenOptions.ClientID)
exec.Args = append(exec.Args, argClientID, o.TokenOptions.ClientID)
}

if o.isSet(flagTenantID) {
exec.Args = append(exec.Args, argTenantID)
exec.Args = append(exec.Args, o.TokenOptions.TenantID)
exec.Args = append(exec.Args, argTenantID, o.TokenOptions.TenantID)
}

if o.isSet(flagAuthorityHost) {
exec.Args = append(exec.Args, argAuthorityHost)
exec.Args = append(exec.Args, o.TokenOptions.AuthorityHost)
exec.Args = append(exec.Args, argAuthorityHost, o.TokenOptions.AuthorityHost)
}

if o.isSet(flagFederatedTokenFile) {
exec.Args = append(exec.Args, argFederatedTokenFile)
exec.Args = append(exec.Args, o.TokenOptions.FederatedTokenFile)
exec.Args = append(exec.Args, argFederatedTokenFile, o.TokenOptions.FederatedTokenFile)
}
}

Expand Down
14 changes: 6 additions & 8 deletions pkg/token/azurecli.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,20 @@ import (
)

type AzureCLIToken struct {
resourceID string
tenantID string
oAuthConfig adal.OAuthConfig
resourceID string
tenantID string
}

// newAzureCLIToken returns a TokenProvider that will fetch a token for the user currently logged into the Azure CLI.
// Required arguments include an oAuthConfiguration object and the resourceID (which is used as the scope)
func newAzureCLIToken(oAuthConfig adal.OAuthConfig, resourceID string, tenantID string) (TokenProvider, error) {
func newAzureCLIToken(resourceID string, tenantID string) (TokenProvider, error) {
if resourceID == "" {
return nil, errors.New("resourceID cannot be empty")
}

return &AzureCLIToken{
resourceID: resourceID,
oAuthConfig: oAuthConfig,
tenantID: tenantID,
resourceID: resourceID,
tenantID: tenantID,
}, nil
}

Expand All @@ -49,7 +47,7 @@ func (p *AzureCLIToken) Token() (adal.Token, error) {
if err != nil {
return emptyToken, fmt.Errorf("expected an empty error but received: %v", err)
}
if len(cliAccessToken.Token) == 0 {
if cliAccessToken.Token == "" {
return emptyToken, errors.New("did not receive a token")
}

Expand Down
7 changes: 3 additions & 4 deletions pkg/token/execCredentialWriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (*execCredentialWriter) Write(token adal.Token, writer io.Writer) error {
if err != nil {
return err
}
//Support both apiVersions of client.authentication.k8s.io/v1beta1 and client.authentication.k8s.io/v1
// Support both apiVersions of client.authentication.k8s.io/v1beta1 and client.authentication.k8s.io/v1
var ec interface{}
t := metav1.NewTime(token.Expires())
switch apiVersionFromEnv {
Expand Down Expand Up @@ -74,9 +74,8 @@ func getAPIVersionFromExecInfoEnv() (string, error) {
return apiV1beta1, nil
}
var execCredential clientauthentication.ExecCredential
error := json.Unmarshal([]byte(env), &execCredential)
if error != nil {
return "", fmt.Errorf("cannot unmarshal %q to ExecCredential: %w", env, error)
if err := json.Unmarshal([]byte(env), &execCredential); err != nil {
return "", fmt.Errorf("cannot unmarshal %q to ExecCredential: %w", env, err)
}
switch execCredential.TypeMeta.APIVersion {
case "":
Expand Down
2 changes: 1 addition & 1 deletion pkg/token/interactive.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (p *InteractiveToken) Token() (adal.Token, error) {
if err != nil {
return emptyToken, fmt.Errorf("expected an empty error but received: %w", err)
}
if len(interactiveToken.Token) == 0 {
if interactiveToken.Token == "" {
return emptyToken, errors.New("did not receive a token")
}

Expand Down
Loading

0 comments on commit 992e74d

Please sign in to comment.