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.

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

Lint code and bump golangci-lint linter #13

Merged
merged 3 commits into from
Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions .github/workflows/integration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ jobs:
steps:
- uses: actions/checkout@v2
- name: Run golangci-lint
uses: golangci/golangci-lint-action@v3.2.0
uses: golangci/golangci-lint-action@3a919529898de77ec3da873e3063ca4b10e7f5cc # v3.7.0
with:
version: v1.50.0
version: v1.55.2
only-new-issues: false
args: --config .golangci.yml --timeout=5m

2 changes: 1 addition & 1 deletion .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
- name: Setup Go
uses: actions/setup-go@84cbf8094393cdc5fe1fe1671ff2647332956b1a #v3.2.1
with:
go-version: '1.20'
go-version-file: 'go.mod'
Copy link
Contributor

Choose a reason for hiding this comment

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

Neat! Now I want this in all repos!


- uses: sigstore/cosign-installer@11086d25041f77fe8fe7b9ea4e48e3b9192b8f19 #v3.1.2

Expand Down
15 changes: 11 additions & 4 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
linters-settings:
govet:
check-shadowing: true
cyclop:
max-complexity: 15
maligned:
suggest-new: true
goconst:
Expand All @@ -10,7 +12,7 @@ linters-settings:
sections:
- standard # Captures all standard packages if they do not match another section.
- default # Contains all imports that could not be matched to another section type.
- prefix(github.com/falcosecurity/peribolos-owners-syncer) # Groups all imports with the specified Prefix.
- prefix(github.com/falcosecurity/peribolos-syncer) # Groups all imports with the specified Prefix.
tagliatelle:
case:
rules:
Expand All @@ -31,14 +33,19 @@ linters:
- ireturn
- lll
- nonamedreturns
- wrapcheck
- varnamelen
- depguard
- wrapcheck

issues:
exclude-rules:
- path: /
linters:
- typecheck
- path: _test.go
linters:
- revive
- ineffassign
- staticcheck
- errcheck

run:
skip-dirs: []
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ lint: golangci-lint

.PHONY: golangci-lint
golangci-lint:
@$(go) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.50.0
@$(go) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.55.2

.PHONY: gofumpt
gofumpt:
Expand Down
2 changes: 2 additions & 0 deletions cmd/sync/github/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,6 @@ peribolos-syncer sync github --org=acme --team=app-maintainers
syncerSignature = "Autogenerated with [peribolos-syncer](https://github.com/falcosecurity/peribolos-syncer)."
ownersDoc = "https://docs.prow.k8s.io/docs/components/plugins/approve/approvers/#overview"
peribolosConfigFile = "org.yaml"

modeConfigFile = 0o644
)
18 changes: 10 additions & 8 deletions cmd/sync/github/sync_github.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type options struct {
publicGPGKeyPath string

github syncergithub.GitHubOptions
orgs *orgs.PeribolosOptions
orgs *orgs.Options
owners *owners.OwnersLoadingOptions

git.ListOptions
Expand All @@ -60,7 +60,7 @@ func New() *cobra.Command {
author: gitobject.Signature{},
github: syncergithub.GitHubOptions{},
owners: &owners.OwnersLoadingOptions{},
orgs: &orgs.PeribolosOptions{},
orgs: &orgs.Options{},
}

cmd := &cobra.Command{
Expand Down Expand Up @@ -94,19 +94,19 @@ func New() *cobra.Command {

func (o *options) validate() error {
if o.GitHubOrg == "" {
return fmt.Errorf("github organization name is empty")
return errors.New("github organization name is empty")
}

if o.GitHubTeam == "" {
return fmt.Errorf("github team name is empty")
return errors.New("github team name is empty")
}

if o.author.Name == "" {
return fmt.Errorf("git author name is empty")
return errors.New("git author name is empty")
}

if o.author.Email == "" {
return fmt.Errorf("git author email is empty")
return errors.New("git author email is empty")
}

if o.publicGPGKeyPath == "" {
Expand All @@ -132,6 +132,7 @@ func (o *options) validate() error {
return nil
}

//nolint:funlen
func (o *options) Run(_ *cobra.Command, _ []string) error {
if err := o.validate(); err != nil {
return err
Expand Down Expand Up @@ -178,7 +179,7 @@ func (o *options) Run(_ *cobra.Command, _ []string) error {
}

// Synchronize the Github Team config with Approvers.
if err := orgs.AddTeamMembers(config, o.GitHubOrg, o.GitHubTeam, people); err != nil {
if err = orgs.AddTeamMembers(config, o.GitHubOrg, o.GitHubTeam, people); err != nil {
return errors.Wrap(err, "error updating maintainers github team from leaf approvers")
}

Expand Down Expand Up @@ -272,6 +273,7 @@ func (o *options) loadPeopleFromOwners(owners repoowners.RepoOwner) []string {
switch {
// Limiting the scope of the roles.
case o.owners.ConfigPath != "":
//nolint:gocritic
if o.owners.ApproversOnly {
// Approvers of the subpart of the repository.
people = maps.Keys(owners.Approvers(o.owners.ConfigPath).Set())
Expand Down Expand Up @@ -303,7 +305,7 @@ func (o *options) flushConfig(config *peribolos.FullConfig, configPath string) e
return errors.Wrap(err, "error recompiling the peribolos config")
}

if err = os.WriteFile(path.Join(configPath, o.orgs.ConfigPath), b, 0644); err != nil {
if err = os.WriteFile(path.Join(configPath, o.orgs.ConfigPath), b, modeConfigFile); err != nil {
return errors.Wrap(err, "error writing the recompiled peribolos config")
}

Expand Down
33 changes: 17 additions & 16 deletions cmd/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,21 @@ import (
"github.com/falcosecurity/peribolos-syncer/internal/output"
)

var (
// Semantic version that refers to ghe version (git tag) of syncer that is released.
const (
// Semantic Version that refers to ghe Version (git tag) of syncer that is released.
// NOTE: The $Format strings are replaced during 'git archive' thanks to the
// companion .gitattributes file containing 'export-subst' in this same
// directory. See also https://git-scm.com/docs/gitattributes.
semVersion = "v0.0.0-master+$Format:%H$"
SemVersion = "v0.0.0-master+$Format:%H$"

// sha1 from git, output of $(git rev-parse HEAD).
gitCommit = "$Format:%H$"
GitCommit = "$Format:%H$"

// build date in ISO8601 format, output of $(date -u +'%Y-%m-%dT%H:%M:%SZ').
buildDate = "1970-01-01T00:00:00Z"
BuildDate = "1970-01-01T00:00:00Z"
)

type version struct {
type Version struct {
SemVersion string `json:"sem_version"`
GitCommit string `json:"git_commit"`
BuildDate string `json:"build_date"`
Expand All @@ -46,13 +46,13 @@ type version struct {
Platform string `json:"platform"`
}

// New returns a new version command.
// New returns a new Version command.
func New() *cobra.Command {
v := newVersion()
v := NewVersion()

cmd := &cobra.Command{
Use: "version",
Short: "Return the syncer version",
Use: "Version",
Short: "Return the syncer Version",
RunE: func(cmd *cobra.Command, args []string) error {
return v.Print()
},
Expand All @@ -61,18 +61,19 @@ func New() *cobra.Command {
return cmd
}

func (o *version) Print() error {
func (o *Version) Print() error {
output.Print(o.SemVersion)

return nil
}

func newVersion() version {
func NewVersion() Version {
// These variables usually come from -ldflags settings and in their
// absence fallback to the ones defined in the var section.
return version{
SemVersion: semVersion,
GitCommit: gitCommit,
BuildDate: buildDate,
return Version{
SemVersion: SemVersion,
GitCommit: GitCommit,
BuildDate: BuildDate,
GoVersion: runtime.Version(),
Compiler: runtime.Compiler,
Platform: fmt.Sprintf("%s/%s", runtime.GOOS, runtime.GOARCH),
Expand Down
20 changes: 11 additions & 9 deletions cmd/version/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package version
package version_test

import (
"fmt"
Expand All @@ -21,10 +21,12 @@ import (

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

cmd "github.com/falcosecurity/peribolos-syncer/cmd/version"
)

var _ = Describe("Version", func() {
version := &version{
version := &cmd.Version{
SemVersion: "0.0.0",
GitCommit: "ef30d58",
BuildDate: "2024-03-25_17:55:07",
Expand All @@ -33,19 +35,19 @@ var _ = Describe("Version", func() {
Platform: "linux/test",
}

Context("testing version Print function", func() {
Context("testing Version Print function", func() {
It("should not error", func() {
Expect(version.Print()).Error().ShouldNot(HaveOccurred())
})
})

Context("testing newVersion function", func() {
It("should return the expected version struct", func() {
v := newVersion()
Context("testing NewVersion function", func() {
It("should return the expected Version struct", func() {
v := cmd.NewVersion()
Expect(v.Compiler).Should(Equal(runtime.Compiler))
Expect(v.SemVersion).Should(Equal(semVersion))
Expect(v.GitCommit).Should(Equal(gitCommit))
Expect(v.BuildDate).Should(Equal(buildDate))
Expect(v.SemVersion).Should(Equal(cmd.SemVersion))
Expect(v.GitCommit).Should(Equal(cmd.GitCommit))
Expect(v.BuildDate).Should(Equal(cmd.BuildDate))
Expect(v.GoVersion).Should(Equal(runtime.Version()))
Expect(v.Platform).Should(Equal(fmt.Sprintf("%s/%s", runtime.GOOS, runtime.GOARCH)))
})
Expand Down
8 changes: 5 additions & 3 deletions internal/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,21 @@
package git

import (
gitobject "github.com/go-git/go-git/v5/plumbing/object"
"time"

"github.com/ProtonMail/go-crypto/openpgp"

"github.com/go-git/go-git/v5"
gitplumbing "github.com/go-git/go-git/v5/plumbing"
gitobject "github.com/go-git/go-git/v5/plumbing/object"
"github.com/google/uuid"
"github.com/pkg/errors"
)

func NewEphemeralGitBranch(repo *git.Repository, worktree *git.Worktree) (string, error) {
id := uuid.New()

refName := id.String()

headRef, err := repo.Head()
if err != nil {
return "", errors.Wrap(err, "error getting repository HEAD reference")
Expand All @@ -52,7 +53,8 @@ func NewEphemeralGitBranch(repo *git.Repository, worktree *git.Worktree) (string
}

func StageAndCommit(repo *git.Repository, worktree *git.Worktree, author *gitobject.Signature,
pgpEntity *openpgp.Entity, stagePath, commitMessage string) error {
pgpEntity *openpgp.Entity, stagePath, commitMessage string,
) error {
if worktree == nil {
return errors.New("worktree cannot be empty")
}
Expand Down
6 changes: 2 additions & 4 deletions internal/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

"github.com/go-git/go-git/v5"
githttp "github.com/go-git/go-git/v5/plumbing/transport/http"

"github.com/pkg/errors"
"github.com/spf13/pflag"
"k8s.io/test-infra/pkg/flagutil"
Expand Down Expand Up @@ -76,10 +75,9 @@ func (o *GitHubOptions) GetGitClientFactory() (gitv2.ClientFactory, error) {
return factory, nil
}

func (o *GitHubOptions) ForkRepository(githubClient prowgithub.Client,
githubOrg, githubRepo, token string) (*git.Repository, *git.Worktree, string, error) {

func (o *GitHubOptions) ForkRepository(githubClient prowgithub.Client, githubOrg, githubRepo, token string) (*git.Repository, *git.Worktree, string, error) {
githubClient.Used()

path, err := os.MkdirTemp("", "orgs")
if err != nil {
return nil, nil, "", errors.Wrap(err, "error creating temporary directory for cloning git repository")
Expand Down
4 changes: 2 additions & 2 deletions internal/owners/owners.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ type Handle string
//
//nolint:revive
type OwnersLoadingOptions struct {

// RepositoryName represents the git repository name at which load the Owners config.
RepositoryName string

Expand Down Expand Up @@ -74,7 +73,8 @@ func (o *OwnersLoadingOptions) AddPFlags(pfs *pflag.FlagSet) {
// It wraps around repoowners.NewClient facilitating the client build configuration with default settings.
// It possibly returns an error.
func NewClient(githubClient github.Client,
gitClientFactory gitv2.ClientFactory) *repoowners.Client {
gitClientFactory gitv2.ClientFactory,
) *repoowners.Client {
mdYAMLEnabled := func(org, repo string) bool {
return false
}
Expand Down
6 changes: 0 additions & 6 deletions internal/owners/owners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ import (
. "github.com/falcosecurity/peribolos-syncer/internal/owners"
)

const (
repo = "peribolos-syncer"
ref = "main"
ownersPath = "OWNERS"
)

var _ = Describe("Creating new client", func() {
var (
ownersClient *repoowners.Client
Expand Down
1 change: 0 additions & 1 deletion pkg/peribolos/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ func AddTeamMaintainers(config *peribolos.FullConfig, org, team string, maintain
func AddTeamMembers(config *peribolos.FullConfig, org, team string, members []string) error {
orgConfig, ok := config.Orgs[org]
if !ok {
//nolint:goerr113
return errors.New("organization not found in peribolos config")
}

Expand Down
Loading
Loading