diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..a00b8fc4 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,121 @@ +linters-settings: + depguard: + list-type: blacklist + packages: + - github.com/jenkins-x/jx/v2/pkg/log/ + - github.com/satori/go.uuid + - github.com/pborman/uuid + packages-with-error-message: + - github.com/jenkins-x/jx/v2/pkg/log/: "use jenkins-x/jx-logging instead" + - github.com/satori/go.uuid: "use github.com/google/uuid instead" + - github.com/pborman/uuid: "use github.com/google/uuid instead" + dupl: + threshold: 100 + exhaustive: + default-signifies-exhaustive: false + funlen: + lines: 200 + statements: 150 + goconst: + min-len: 3 + min-occurrences: 3 + gocritic: + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + disabled-checks: + - dupImport # https://github.com/go-critic/go-critic/issues/845 + - ifElseChain + - octalLiteral + - whyNoLint + - wrapperFunc + - importShadow # not important for now + - unnamedResult # not important + gocyclo: + min-complexity: 15 + goimports: {} + golint: + min-confidence: 0 + gofmt: + simplify: true + gomnd: + settings: + mnd: + # don't include the "operation" and "assign" + checks: [argument, case, condition, return] + govet: + check-shadowing: true + settings: + printf: + funcs: + - (github.com/jenkins-x/jx-logging/v3/pkg/log/Logger()).Debugf + - (github.com/jenkins-x/jx-logging/v3/pkg/log/Logger()).Infof + - (github.com/jenkins-x/jx-logging/v3/pkg/log/Logger()).Warnf + - (github.com/jenkins-x/jx-logging/v3/pkg/log/Logger()).Errorf + - (github.com/jenkins-x/jx-logging/v3/pkg/log/Logger()).Fatalf + lll: + line-length: 140 + maligned: + suggest-new: true + misspell: {} + nolintlint: + allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space) + allow-unused: false # report any unused nolint directives + require-explanation: false # don't require an explanation for nolint directives + require-specific: false # don't require nolint directives to be specific about which linter is being skipped +linters: + # please, do not use `enable-all`: it's deprecated and will be removed soon. + # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint + disable-all: true + enable: + - asciicheck + - bodyclose + - deadcode + - depguard + - errcheck + - gofmt + - goimports + - goprintffuncname + - gosec + - gosimple + - ineffassign + - misspell + - nakedret + - rowserrcheck + - staticcheck + - structcheck + - typecheck + - unconvert + - unparam + - unused + - varcheck + - revive + - gocritic + - govet +issues: + # Excluding configuration per-path, per-linter, per-text and per-source + exclude-rules: + # - path: _test\.go + # linters: + # - gomnd + # https://github.com/go-critic/go-critic/issues/926 + - linters: + - gocritic + text: "unnecessaryDefer:" + exclude: + - 'shadow: declaration of "err" shadows declaration at' + max-same-issues: 0 + +run: + timeout: 30m + skip-dirs: + - cmd/docs +# golangci.com configuration +# https://github.com/golangci/golangci/wiki/Configuration +service: + golangci-lint-version: 1.42.x # use the fixed version to not introduce new linters unexpectedly + prepare: + - echo "here I can run custom commands, but no preparation needed for this repo" diff --git a/.lighthouse/jenkins-x/lint.yaml b/.lighthouse/jenkins-x/lint.yaml new file mode 100644 index 00000000..52a721f8 --- /dev/null +++ b/.lighthouse/jenkins-x/lint.yaml @@ -0,0 +1,30 @@ +apiVersion: tekton.dev/v1beta1 +kind: PipelineRun +metadata: + creationTimestamp: null + name: lint +spec: + pipelineSpec: + tasks: + - name: jx-promote-lint + resources: {} + taskSpec: + metadata: {} + stepTemplate: + image: uses:jenkins-x/jx3-pipeline-catalog/tasks/go/pullrequest.yaml@versionStream + name: "" + resources: + requests: + cpu: 400m + memory: 600Mi + workingDir: /workspace/source + steps: + - image: uses:jenkins-x/jx3-pipeline-catalog/tasks/git-clone/git-clone-pr.yaml@versionStream + name: "" + resources: {} + - name: make-lint + resources: {} + podTemplate: {} + serviceAccountName: tekton-bot + timeout: 30m0s +status: {} diff --git a/.lighthouse/jenkins-x/triggers.yaml b/.lighthouse/jenkins-x/triggers.yaml index d955caf1..d4ccfeab 100644 --- a/.lighthouse/jenkins-x/triggers.yaml +++ b/.lighthouse/jenkins-x/triggers.yaml @@ -6,6 +6,13 @@ spec: context: "pr" always_run: true optional: false - trigger: "/test" - rerun_command: "/retest" + trigger: (?m)^/test( all| pr),?(s+|$) + rerun_command: /test pr source: "pullrequest.yaml" + - name: lint + context: "lint" + always_run: true + optional: false + trigger: (?m)^/test( all| lint),?(s+|$) + rerun_command: /test lint + source: "lint.yaml" diff --git a/cmd/app/main.go b/cmd/app/main.go index 071e1a2c..b99edcb3 100644 --- a/cmd/app/main.go +++ b/cmd/app/main.go @@ -1,3 +1,4 @@ +//nolint // +build !windows package app diff --git a/hack/linter.sh b/hack/linter.sh index 9513303d..cdeceac5 100755 --- a/hack/linter.sh +++ b/hack/linter.sh @@ -11,36 +11,10 @@ DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" if ! [ -x "$(command -v golangci-lint)" ]; then echo "Installing GolangCI-Lint" - ${DIR}/install_golint.sh -b $GOPATH/bin v1.20.0 + ${DIR}/install_golint.sh -b $GOPATH/bin v1.42.1 fi export GO111MODULE=on golangci-lint run \ - --no-config \ - --disable-all \ - -E golint \ - -E varcheck \ - -E errcheck \ - -E misspell \ - -E unconvert \ - -E deadcode \ - -E unconvert \ - -E gosec \ - -E gofmt \ - -E goimports \ - -E structcheck \ - -E interfacer \ - -E govet \ - -E unparam \ - -E megacheck \ - -E goconst \ - -E ineffassign \ - -E unparam \ - -E gocritic \ - -E maligned \ - -E typecheck \ - --skip-dirs vendor \ - --deadline 15m0s \ --verbose \ --build-tags build - diff --git a/pkg/envctx/env_context.go b/pkg/envctx/env_context.go index bffd96d7..fb76093e 100644 --- a/pkg/envctx/env_context.go +++ b/pkg/envctx/env_context.go @@ -56,7 +56,7 @@ type ChartDetails struct { // ChartDetails resolves the chart details from a full or local name and an optional repository URL. // this function can handle an empty repository but the chart name "foo/bar" and resolve the prefix "foo" to a repository // URL - or taking chart name "bar" and a repository URL and defaulting the prefix to "foo/bar" -func (c *EnvironmentContext) ChartDetails(chartName string, repo string) (*ChartDetails, error) { +func (c *EnvironmentContext) ChartDetails(chartName, repo string) (*ChartDetails, error) { prefix := "" localName := chartName name := chartName diff --git a/pkg/envctx/load.go b/pkg/envctx/load.go index ac1f770f..050ddee7 100644 --- a/pkg/envctx/load.go +++ b/pkg/envctx/load.go @@ -107,7 +107,6 @@ func (e *EnvironmentContext) LazyLoad(gclient gitclient.Interface, jxClient vers if err != nil { return errors.Wrapf(err, "failed to create version stream dir %s", versionsDir) } - //return errors.Errorf("dev environment git repository %s does not have a versionStream dir", url) } e.VersionResolver = &versionstream.VersionResolver{ diff --git a/pkg/environments/fork.go b/pkg/environments/fork.go index 89ffa7bc..7e5121cb 100644 --- a/pkg/environments/fork.go +++ b/pkg/environments/fork.go @@ -2,6 +2,7 @@ package environments import ( "context" + "github.com/jenkins-x/go-scm/scm" "github.com/jenkins-x/jx-helpers/v3/pkg/gitclient" "github.com/jenkins-x/jx-helpers/v3/pkg/scmhelpers" @@ -40,7 +41,7 @@ func (o *EnvironmentPullRequestOptions) EnsureForked(client *scm.Client, repoNam return repo.Clone, nil } -func (o *EnvironmentPullRequestOptions) rebaseForkFromUpstream(dir string, gitURL string) error { +func (o *EnvironmentPullRequestOptions) rebaseForkFromUpstream(dir, gitURL string) error { g := o.Git() branch, err := gitclient.Branch(g, dir) if err != nil { diff --git a/pkg/environments/fork_test.go b/pkg/environments/fork_test.go index 9ece0581..1a919dbd 100644 --- a/pkg/environments/fork_test.go +++ b/pkg/environments/fork_test.go @@ -2,12 +2,13 @@ package environments_test import ( "context" + "testing" + "github.com/jenkins-x-plugins/jx-promote/pkg/environments" "github.com/jenkins-x/go-scm/scm" "github.com/jenkins-x/go-scm/scm/driver/fake" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "testing" ) func TestFork(t *testing.T) { diff --git a/pkg/environments/gitops.go b/pkg/environments/gitops.go index 4d882f68..93f53e1f 100644 --- a/pkg/environments/gitops.go +++ b/pkg/environments/gitops.go @@ -48,7 +48,6 @@ func (o *EnvironmentPullRequestOptions) Create(gitURL, prDir string, pullRequest if err != nil { return nil, err } - prDir = tempDir defer os.RemoveAll(tempDir) } diff --git a/pkg/environments/pr.go b/pkg/environments/pr.go index 1c6c6d2c..9ca4c8cc 100644 --- a/pkg/environments/pr.go +++ b/pkg/environments/pr.go @@ -32,7 +32,7 @@ func (o *EnvironmentPullRequestOptions) Git() gitclient.Interface { } // CreatePullRequest crates a pull request if there are git changes -func (o *EnvironmentPullRequestOptions) CreatePullRequest(scmClient *scm.Client, gitURL string, repoFullName, dir string, doneCommit bool, existingPR *scm.PullRequest) (*scm.PullRequest, error) { +func (o *EnvironmentPullRequestOptions) CreatePullRequest(scmClient *scm.Client, gitURL, repoFullName, dir string, doneCommit bool, existingPR *scm.PullRequest) (*scm.PullRequest, error) { gitter := o.Git() changes, err := gitclient.HasChanges(gitter, dir) if err != nil { @@ -150,7 +150,7 @@ func (o *EnvironmentPullRequestOptions) CreatePullRequest(scmClient *scm.Client, return o.addLabelsToPullRequest(ctx, scmClient, repoFullName, pr) } -func (o *EnvironmentPullRequestOptions) GetScmClient(gitURL string, kind string) (*scm.Client, string, error) { +func (o *EnvironmentPullRequestOptions) GetScmClient(gitURL, kind string) (*scm.Client, string, error) { if gitURL == "" { log.Logger().Infof("no git URL specified so cannot create a Pull Request") return nil, "", nil diff --git a/pkg/environments/types.go b/pkg/environments/types.go index 5b4e0d23..9b7a09f7 100644 --- a/pkg/environments/types.go +++ b/pkg/environments/types.go @@ -12,7 +12,7 @@ import ( "helm.sh/helm/v3/pkg/chart" ) -//ValuesFiles is a wrapper for a slice of values files to allow them to be passed around as a pointer +// ValuesFiles is a wrapper for a slice of values files to allow them to be passed around as a pointer type ValuesFiles struct { Items []string } diff --git a/pkg/promote/promote.go b/pkg/promote/promote.go index 4d28b888..7a15b4b5 100644 --- a/pkg/promote/promote.go +++ b/pkg/promote/promote.go @@ -64,9 +64,7 @@ const ( DefaultChartRepo = "http://jenkins-x-chartmuseum:8080" ) -var ( - waitAfterPullRequestCreated = time.Second * 3 -) +var waitAfterPullRequestCreated = time.Second * 3 // Options containers the CLI options type Options struct { @@ -361,6 +359,9 @@ func (o *Options) Run() error { if o.Version == "" && o.Application != "" { if o.Interactive { versions, err := o.getAllVersions(o.Application) + if err != nil { + return errors.Wrap(err, "failed to get app versions") + } o.Version, err = o.Input.PickNameWithDefault(versions, "Pick version:", "", "please select a version") if err != nil { return errors.Wrapf(err, "failed to pick a version") @@ -371,7 +372,6 @@ func (o *Options) Run() error { return errors.Wrapf(err, "failed to find latest version of app %s", o.Application) } } - } ns := o.Namespace @@ -422,14 +422,14 @@ func (o *Options) Run() error { if o.PullRequestPollTime != "" { duration, err := time.ParseDuration(o.PullRequestPollTime) if err != nil { - return fmt.Errorf("Invalid duration format %s for option --%s: %s", o.PullRequestPollTime, optionPullRequestPollTime, err) + return fmt.Errorf("invalid duration format %s for option --%s: %s", o.PullRequestPollTime, optionPullRequestPollTime, err) } o.PullRequestPollDuration = &duration } if o.Timeout != "" { duration, err := time.ParseDuration(o.Timeout) if err != nil { - return fmt.Errorf("Invalid duration format %s for option --%s: %s", o.Timeout, optionTimeout, err) + return fmt.Errorf("invalid duration format %s for option --%s: %s", o.Timeout, optionTimeout, err) } o.TimeoutDuration = &duration } @@ -470,7 +470,7 @@ func (o *Options) Run() error { return err } if env == nil { - return fmt.Errorf("Could not find an Environment called %s", o.Environment) + return fmt.Errorf("could not find an Environment called %s", o.Environment) } } releaseInfo, err := o.Promote([]*jxcore.EnvironmentConfig{env}, true, o.NoPoll) @@ -683,7 +683,7 @@ func (o *Options) Promote(envs []*jxcore.EnvironmentConfig, warnIfAuto, noPoll b draftPR := strategy != v1.PromotionStrategyTypeAutomatic targetNS := EnvironmentNamespace(env) if targetNS == "" { - return nil, fmt.Errorf("No namespace for environment %s", env.Key) + return nil, fmt.Errorf("no namespace for environment %s", env.Key) } if warnIfAuto && env != nil && strategy == v1.PromotionStrategyTypeAutomatic && !o.BatchMode { @@ -714,7 +714,10 @@ func (o *Options) Promote(envs []*jxcore.EnvironmentConfig, warnIfAuto, noPoll b err := o.PromoteViaPullRequest(envs, releaseInfo, draftPR) if err == nil { startPromotePR := func(a *v1.PipelineActivity, s *v1.PipelineActivityStep, ps *v1.PromoteActivityStep, p *v1.PromotePullRequestStep) error { - activities.StartPromotionPullRequest(a, s, ps, p) + err = activities.StartPromotionPullRequest(a, s, ps, p) + if err != nil { + return err + } pr := releaseInfo.PullRequestInfo if pr != nil && pr.Link != "" { p.PullRequestURL = pr.Link @@ -768,7 +771,6 @@ func (o *Options) ResolveChartRepositoryURL() (string, error) { } if chartRepo == "" { log.Logger().Warnf("no cluster.chartRepository in your jx-requirements.yml in your cluster so trying to discover from kubernetes ingress and service resources") - } else { log.Logger().Warnf("the cluster.chartRepository in your jx-requirements.yml looks like its an internal service URL so trying to discover from kubernetes ingress resources") } @@ -827,10 +829,10 @@ func IsLocalChartRepository(repo string) bool { return !strings.Contains(repo, ".") } -func (o *Options) GetTargetNamespace(ns string, env string) (string, *jxcore.EnvironmentConfig, error) { +func (o *Options) GetTargetNamespace(ns, env string) (string, *jxcore.EnvironmentConfig, error) { environments := o.DevEnvContext.Requirements.Environments if len(environments) == 0 { - return "", nil, fmt.Errorf("No Environments have been defined in the requirements and settings files") + return "", nil, fmt.Errorf("no Environments have been defined in the requirements and settings files") } var envResource *jxcore.EnvironmentConfig @@ -840,14 +842,14 @@ func (o *Options) GetTargetNamespace(ns string, env string) (string, *jxcore.Env envResource, err = o.DevEnvContext.Requirements.Environment(env) if envResource == nil || err != nil { var envNames []string - for _, e := range environments { - envNames = append(envNames, e.Key) + for k := range environments { + envNames = append(envNames, environments[k].Key) } return "", nil, options.InvalidOption(optionEnvironment, env, envNames) } targetNS = EnvironmentNamespace(envResource) if targetNS == "" { - return "", nil, fmt.Errorf("environment %s does not have a namespace associated with it!", env) + return "", nil, fmt.Errorf("environment %s does not have a namespace associated with it", env) } } else if ns != "" { targetNS = ns @@ -885,7 +887,10 @@ func (o *Options) WaitForPromotion(ns string, env *jxcore.EnvironmentConfig, rel err := o.waitForGitOpsPullRequest(ns, env, releaseInfo, end, duration, promoteKey) if err != nil { // TODO based on if the PR completed or not fail the PR or the Promote? - promoteKey.OnPromotePullRequest(kubeClient, jxClient, o.Namespace, activities.FailedPromotionPullRequest) + err2 := promoteKey.OnPromotePullRequest(kubeClient, jxClient, o.Namespace, activities.FailedPromotionPullRequest) + if err2 != nil { + return err2 + } return err } } @@ -935,15 +940,20 @@ func (o *Options) waitForGitOpsPullRequest(ns string, env *jxcore.EnvironmentCon } else { mergeSha := pr.MergeSha if !logHasMergeSha { - logHasMergeSha = true log.Logger().Infof("Pull Request %s is merged at sha %s", termcolor.ColorInfo(pr.Link), termcolor.ColorInfo(mergeSha)) mergedPR := func(a *v1.PipelineActivity, s *v1.PipelineActivityStep, ps *v1.PromoteActivityStep, p *v1.PromotePullRequestStep) error { - activities.CompletePromotionPullRequest(a, s, ps, p) + err = activities.CompletePromotionPullRequest(a, s, ps, p) + if err != nil { + return err + } p.MergeCommitSHA = mergeSha return nil } - promoteKey.OnPromotePullRequest(kubeClient, jxClient, o.Namespace, mergedPR) + err = promoteKey.OnPromotePullRequest(kubeClient, jxClient, o.Namespace, mergedPR) + if err != nil { + return err + } if o.NoWaitAfterMerge { log.Logger().Infof("Pull requests are merged, No wait on promotion to complete") @@ -951,7 +961,10 @@ func (o *Options) waitForGitOpsPullRequest(ns string, env *jxcore.EnvironmentCon } } - promoteKey.OnPromoteUpdate(kubeClient, jxClient, o.Namespace, activities.StartPromotionUpdate) + err = promoteKey.OnPromoteUpdate(kubeClient, jxClient, o.Namespace, activities.StartPromotionUpdate) + if err != nil { + return err + } err = o.CommentOnIssues(ns, env, promoteKey) if err == nil { @@ -962,7 +975,7 @@ func (o *Options) waitForGitOpsPullRequest(ns string, env *jxcore.EnvironmentCon } else { if pr.Closed { log.Logger().Warnf("Pull Request %s is closed", termcolor.ColorInfo(pr.Link)) - return fmt.Errorf("Promotion failed as Pull Request %s is closed without merging", pr.Link) + return fmt.Errorf("promotion failed as Pull Request %s is closed without merging", pr.Link) } prLastCommitSha := o.pullRequestLastCommitSha(pr) @@ -970,8 +983,8 @@ func (o *Options) waitForGitOpsPullRequest(ns string, env *jxcore.EnvironmentCon status, err := o.PullRequestLastCommitStatus(pr) if err != nil || status == nil { log.Logger().Warnf("Failed to query the Pull Request last commit status for %s ref %s %s", pr.Link, prLastCommitSha, err) - //return fmt.Errorf("Failed to query the Pull Request last commit status for %s ref %s %s", pr.Link, prLastCommitSha, err) - //} else if status.State == "in-progress" { + // return fmt.Errorf("Failed to query the Pull Request last commit status for %s ref %s %s", pr.Link, prLastCommitSha, err) + // } else if status.State == "in-progress" { } else if StateIsPending(status) { log.Logger().Info("The build for the Pull Request last commit is currently in progress.") } else { @@ -996,7 +1009,7 @@ func (o *Options) waitForGitOpsPullRequest(ns string, env *jxcore.EnvironmentCon } _, err = scmClient.PullRequests.Merge(ctx, fullName, prNumber, prMergeOptions) // TODO - //err = gitProvider.MergePullRequest(pr, "jx promote automatically merged promotion PR") + // err = gitProvider.MergePullRequest(pr, "jx promote automatically merged promotion PR") if err != nil { if !logMergeFailure { logMergeFailure = true @@ -1006,7 +1019,7 @@ func (o *Options) waitForGitOpsPullRequest(ns string, env *jxcore.EnvironmentCon } } } else if StateIsErrorOrFailure(status) { - return fmt.Errorf("Pull request %s last commit has status %s for ref %s", pr.Link, status.State.String(), prLastCommitSha) + return fmt.Errorf("pull request %s last commit has status %s for ref %s", pr.Link, status.State.String(), prLastCommitSha) } else { log.Logger().Infof("got git provider status %s from PR %s", status.State.String(), pr.Link) } @@ -1016,13 +1029,13 @@ func (o *Options) waitForGitOpsPullRequest(ns string, env *jxcore.EnvironmentCon log.Logger().Info("Rebasing PullRequest due to conflict") err = o.PromoteViaPullRequest([]*jxcore.EnvironmentConfig{env}, releaseInfo, false) - if releaseInfo.PullRequestInfo != nil { - pullRequestInfo = releaseInfo.PullRequestInfo + if err != nil { + return err } } } if time.Now().After(end) { - return fmt.Errorf("Timed out waiting for pull request %s to merge. Waited %s", pr.Link, duration.String()) + return fmt.Errorf("timed out waiting for pull request %s to merge. Waited %s", pr.Link, duration.String()) } time.Sleep(*o.PullRequestPollDuration) } @@ -1091,10 +1104,8 @@ func (o *Options) findLatestVersion(app string) (string, error) { if maxString == "" || strings.Compare(chart.ChartVersion, maxString) > 0 { maxString = chart.ChartVersion } - } else { - if maxSemVer == nil || maxSemVer.Compare(sv) > 0 { - maxSemVer = &sv - } + } else if maxSemVer == nil || maxSemVer.Compare(sv) > 0 { + maxSemVer = &sv } } @@ -1102,7 +1113,7 @@ func (o *Options) findLatestVersion(app string) (string, error) { return maxSemVer.String(), nil } if maxString == "" { - return "", fmt.Errorf("Could not find a version of app %s in the helm repositories", app) + return "", fmt.Errorf("could not find a version of app %s in the helm repositories", app) } return maxString, nil } @@ -1125,8 +1136,7 @@ func (o *Options) getAllVersions(app string) ([]string, error) { if len(versions) > 0 { return versions, nil } - return nil, fmt.Errorf("Could not find a version of app %s in the helm repositories", app) - + return nil, fmt.Errorf("could not find a version of app %s in the helm repositories", app) } // Helm lazily create a helmer @@ -1213,7 +1223,8 @@ func (o *Options) GetLatestPipelineBuildByCRD(pipeline string) (string, error) { } buildNumber := 0 - for _, p := range pipelines.Items { + for k := range pipelines.Items { + p := pipelines.Items[k] if p.Spec.Pipeline == pipeline { b := p.Spec.Build if b != "" { @@ -1233,7 +1244,7 @@ func (o *Options) GetLatestPipelineBuildByCRD(pipeline string) (string, error) { } // GetPipelineName return the pipeline name -func (o *Options) GetPipelineName(gitInfo *giturl.GitRepository, pipeline string, build string, appName string) (string, string) { +func (o *Options) GetPipelineName(gitInfo *giturl.GitRepository, pipeline, build, appName string) (string, string) { if build == "" { build = builds.GetBuildNumber() } @@ -1263,8 +1274,8 @@ func (o *Options) GetPipelineName(gitInfo *giturl.GitRepository, pipeline string ns := o.Namespace pipelineList, err := jxClient.JenkinsV1().PipelineActivities(ns).List(context.TODO(), metav1.ListOptions{}) if err == nil { - for _, pipelineResource := range pipelineList.Items { - pipelineName := pipelineResource.Spec.Pipeline + for k := range pipelineList.Items { + pipelineName := pipelineList.Items[k].Spec.Pipeline if strings.HasSuffix(pipelineName, suffix) { pipeline = pipelineName break @@ -1340,6 +1351,9 @@ func (o *Options) CommentOnIssues(targetNS string, environment *jxcore.Environme url := "" for _, n := range appNames { url, err = services.FindServiceURL(kubeClient, ens, naming.ToValidName(n)) + if err != nil { + return err + } if url != "" { break } @@ -1356,6 +1370,9 @@ func (o *Options) CommentOnIssues(targetNS string, environment *jxcore.Environme ing, err := kubeClient.ExtensionsV1beta1().Ingresses(ens).Get(context.TODO(), app, metav1.GetOptions{}) if err != nil || ing == nil && o.ReleaseName != "" && o.ReleaseName != app { ing, err = kubeClient.ExtensionsV1beta1().Ingresses(ens).Get(context.TODO(), o.ReleaseName, metav1.GetOptions{}) + if err != nil { + return err + } } if ing != nil { if len(ing.Spec.Rules) > 0 { @@ -1383,7 +1400,8 @@ func (o *Options) CommentOnIssues(targetNS string, environment *jxcore.Environme if release.Spec.ReleaseNotesURL != "" { versionMessage = "[" + version + "](" + release.Spec.ReleaseNotesURL + ")" } - for _, issue := range issues { + for k := range issues { + issue := issues[k] if issue.IsClosed() { log.Logger().Infof("Commenting that issue %s is now in %s", termcolor.ColorInfo(issue.URL), termcolor.ColorInfo(envName)) @@ -1393,17 +1411,15 @@ func (o *Options) CommentOnIssues(targetNS string, environment *jxcore.Environme number, err := strconv.Atoi(id) if err != nil { log.Logger().Warnf("Could not parse issue id %s for URL %s", id, issue.URL) - } else { - if number > 0 { - ctx := context.Background() - fullName := scm.Join(gitInfo.Organisation, gitInfo.Name) - input := &scm.CommentInput{ - Body: comment, - } - _, _, err = o.ScmClient.Issues.CreateComment(ctx, fullName, number, input) - if err != nil { - log.Logger().Warnf("Failed to add comment to issue %s: %s", issue.URL, err) - } + } else if number > 0 { + ctx := context.Background() + fullName := scm.Join(gitInfo.Organisation, gitInfo.Name) + input := &scm.CommentInput{ + Body: comment, + } + _, _, err = o.ScmClient.Issues.CreateComment(ctx, fullName, number, input) + if err != nil { + log.Logger().Warnf("Failed to add comment to issue %s: %s", issue.URL, err) } } } @@ -1420,7 +1436,7 @@ func (o *Options) SearchForChart(filter string) (string, error) { return answer, err } if len(charts) == 0 { - return answer, fmt.Errorf("No charts available for search filter: %s", filter) + return answer, fmt.Errorf("no charts available for search filter: %s", filter) } m := map[string]*helm.ChartSummary{} names := []string{} @@ -1441,7 +1457,7 @@ func (o *Options) SearchForChart(filter string) (string, error) { // TODO now we split the chart into name and repo parts := strings.Split(chartName, "/") if len(parts) != 2 { - return answer, fmt.Errorf("Invalid chart name '%s' was expecting single / character separating repo name and chart name", chartName) + return answer, fmt.Errorf("invalid chart name '%s' was expecting single / character separating repo name and chart name", chartName) } repoName := parts[0] appName := parts[1] @@ -1451,20 +1467,19 @@ func (o *Options) SearchForChart(filter string) (string, error) { return answer, err } - repoUrl := repos[repoName] - if repoUrl == "" { - return answer, fmt.Errorf("Failed to find helm chart repo URL for '%s' when possible values are %s", repoName, stringhelpers.SortedMapKeys(repos)) - + repoURL := repos[repoName] + if repoURL == "" { + return answer, fmt.Errorf("failed to find helm chart repo URL for '%s' when possible values are %s", repoName, stringhelpers.SortedMapKeys(repos)) } o.Version = chart.ChartVersion - o.HelmRepositoryURL = repoUrl + o.HelmRepositoryURL = repoURL return appName, nil } func (o *Options) ChooseChart() (string, error) { appName, err := o.SearchForChart("") if err != nil { - return appName, fmt.Errorf("No charts available") + return appName, fmt.Errorf("no charts available") } o.Version = "" // remove version to choose it later return appName, nil diff --git a/pkg/rules/factory/factory_test.go b/pkg/rules/factory/factory_test.go index aa958638..609fcb1f 100644 --- a/pkg/rules/factory/factory_test.go +++ b/pkg/rules/factory/factory_test.go @@ -37,63 +37,65 @@ func TestRuleFactory(t *testing.T) { ns := "jx" testPromoteNS := "jx" for _, f := range fileSlice { - if f.IsDir() { - name := f.Name() - if name == "jenkins-x-versions" { - continue - } - - dir := filepath.Join(tmpDir, name) - - src := filepath.Join("test_data", name) - err = files.CopyDirOverwrite(src, dir) - require.NoError(t, err, "could not copy source data in %s to %s", src, dir) - - cfg, _, err := promoteconfig.Discover(dir, testPromoteNS) - require.NoError(t, err, "failed to load cfg dir %s", dir) - require.NotNil(t, cfg, "no project cfg found in dir %s", dir) - - err, options := loadOptions(dir) - require.NoError(t, err) - - r := &rules.PromoteRule{ - TemplateContext: rules.TemplateContext{ - GitURL: "https://github.com/myorg/myapp.git", - Version: "1.2.3", - AppName: "myapp", - Namespace: ns, - HelmRepositoryURL: "http://chartmuseum-jx.34.78.195.22.nip.io", - ReleaseName: options.ReleaseName, - }, - Dir: dir, - Config: *cfg, - DevEnvContext: jxtesthelpers.CreateTestDevEnvironmentContext(t, ns), - } - - fn := factory.NewFunction(r) - require.NotNil(t, fn, "failed to create RuleFunction at dir %s", dir) - - err = fn(r) - require.NoError(t, err, "failed to invoke RuleFunction %v at dir %s", fn, dir) - - fileName := ruleFileName(cfg) - target := filepath.Join(dir, fileName) - assert.FileExists(t, target) - - testhelpers.AssertTextFilesEqual(t, filepath.Join(src, fileName+".1.expected"), target, fileName) - - // now lets modify to new version - r.TemplateContext.Version = "1.2.4" - - err = fn(r) - require.NoError(t, err, "failed to run FileRule at dir %s", dir) - - testhelpers.AssertTextFilesEqual(t, filepath.Join(src, fileName+".2.expected"), target, fileName) - - if strings.HasPrefix(name, "helmfile-nested") { - testhelpers.AssertTextFilesEqual(t, filepath.Join(src, "helmfile.yaml.expected"), filepath.Join(dir, "helmfile.yaml"), fileName) - } + if !f.IsDir() { + continue } + name := f.Name() + if name == "jenkins-x-versions" { + continue + } + + dir := filepath.Join(tmpDir, name) + + src := filepath.Join("test_data", name) + err = files.CopyDirOverwrite(src, dir) + require.NoError(t, err, "could not copy source data in %s to %s", src, dir) + + cfg, _, err := promoteconfig.Discover(dir, testPromoteNS) + require.NoError(t, err, "failed to load cfg dir %s", dir) + require.NotNil(t, cfg, "no project cfg found in dir %s", dir) + + options, err := loadOptions(dir) + require.NoError(t, err) + + r := &rules.PromoteRule{ + TemplateContext: rules.TemplateContext{ + GitURL: "https://github.com/myorg/myapp.git", + Version: "1.2.3", + AppName: "myapp", + Namespace: ns, + HelmRepositoryURL: "http://chartmuseum-jx.34.78.195.22.nip.io", + ReleaseName: options.ReleaseName, + }, + Dir: dir, + Config: *cfg, + DevEnvContext: jxtesthelpers.CreateTestDevEnvironmentContext(t, ns), + } + + fn := factory.NewFunction(r) + require.NotNil(t, fn, "failed to create RuleFunction at dir %s", dir) + + err = fn(r) + require.NoError(t, err, "failed to invoke RuleFunction %v at dir %s", fn, dir) + + fileName := ruleFileName(cfg) + target := filepath.Join(dir, fileName) + assert.FileExists(t, target) + + testhelpers.AssertTextFilesEqual(t, filepath.Join(src, fileName+".1.expected"), target, fileName) + + // now lets modify to new version + r.TemplateContext.Version = "1.2.4" + + err = fn(r) + require.NoError(t, err, "failed to run FileRule at dir %s", dir) + + testhelpers.AssertTextFilesEqual(t, filepath.Join(src, fileName+".2.expected"), target, fileName) + + if strings.HasPrefix(name, "helmfile-nested") { + testhelpers.AssertTextFilesEqual(t, filepath.Join(src, "helmfile.yaml.expected"), filepath.Join(dir, "helmfile.yaml"), fileName) + } + } } @@ -111,9 +113,9 @@ func ruleFileName(cfg *v1alpha1.Promote) string { return cfg.Spec.FileRule.Path } -func loadOptions(dir string) (error, *TestOptions) { +func loadOptions(dir string) (*TestOptions, error) { filePath := filepath.Join(dir, "options.yaml") options := &TestOptions{} err := yaml2s.LoadFile(filePath, options) - return err, options + return options, err } diff --git a/pkg/rules/file/file_rule.go b/pkg/rules/file/file_rule.go index 7b45fe24..4ced5108 100644 --- a/pkg/rules/file/file_rule.go +++ b/pkg/rules/file/file_rule.go @@ -145,7 +145,7 @@ func createMatcher(rule *v1alpha1.FileRule, lineMatcher v1alpha1.LineMatcher) (f return nil, errors.Errorf("not supported lime matcher %#v", lineMatcher) } -func evaluateTemplate(r *rules.PromoteRule, templateText string, linePrefix string) (string, error) { +func evaluateTemplate(r *rules.PromoteRule, templateText, linePrefix string) (string, error) { if templateText == "" { return "", nil } diff --git a/pkg/rules/helmfile/helmfile_rule.go b/pkg/rules/helmfile/helmfile_rule.go index 840d4977..b9994888 100644 --- a/pkg/rules/helmfile/helmfile_rule.go +++ b/pkg/rules/helmfile/helmfile_rule.go @@ -36,7 +36,7 @@ func Rule(r *rules.PromoteRule) error { } // ModifyAppsFile modifies the 'jx-apps.yml' file to add/update/remove apps -func modifyHelmfile(r *rules.PromoteRule, rule *v1alpha1.HelmfileRule, file string, promoteNs string) error { +func modifyHelmfile(r *rules.PromoteRule, rule *v1alpha1.HelmfileRule, file, promoteNs string) error { exists, err := files.FileExists(file) if err != nil { return errors.Wrapf(err, "failed to detect if file exists %s", file) @@ -141,7 +141,6 @@ func modifyHelmfileApps(r *rules.PromoteRule, helmfile *state.HelmState, promote release := &helmfile.Releases[i] if release.Name == app || release.Name == details.Name || (r.ReleaseName != "" && release.Name == r.ReleaseName) { release.Version = version - found = true return nil } } @@ -173,7 +172,6 @@ func modifyHelmfileApps(r *rules.PromoteRule, helmfile *state.HelmState, promote release := &helmfile.Releases[i] if (release.Name == app || release.Name == details.Name || (r.ReleaseName != "" && release.Name == r.ReleaseName)) && (release.Namespace == promoteNs || isRemoteEnv) { release.Version = version - found = true return nil } } @@ -211,7 +209,8 @@ func defaultPrefix(appsConfig *state.HelmState, envctx *envctx.EnvironmentContex } prefixes := map[string]string{} urls := map[string]string{} - for _, r := range appsConfig.Repositories { + for k := range appsConfig.Repositories { + r := appsConfig.Repositories[k] if r.URL == d.Repository { found = true r.OCI = oci