Skip to content

Commit

Permalink
Add more lint coverage (#10049)
Browse files Browse the repository at this point in the history
* Add more lint coverage

golanglint-ci doesn't pick up subdirectories with their own go.mod
which left certain directories unlinted. To get around this we can
run golanglint-ci directly against those submodules.
  • Loading branch information
rosstimothy authored Feb 7, 2022
1 parent 69a96d4 commit 896261a
Show file tree
Hide file tree
Showing 12 changed files with 108 additions and 80 deletions.
2 changes: 1 addition & 1 deletion .cloudbuild/scripts/internal/changes/changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func isDocChange(path string) bool {
path = strings.ToLower(path)
return strings.HasPrefix(path, "docs/") ||
strings.HasSuffix(path, ".mdx") ||
strings.HasSuffix(path, ".md") ||
strings.HasSuffix(path, ".md") ||
strings.HasPrefix(path, "rfd/")
}

Expand Down
2 changes: 0 additions & 2 deletions .cloudbuild/scripts/internal/etcd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,4 @@ func Start(ctx context.Context, workspace string, uid, gid int, env ...string) e
return trace.Errorf("Timed out waiting for etcd to start")
}
}

return nil
}
29 changes: 28 additions & 1 deletion .github/workflows/robot/internal/bot/bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,37 @@ import (
"github.com/gravitational/trace"
)

// Client implements the GitHub API.
type Client interface {
// RequestReviewers is used to assign reviewers to a PR.
RequestReviewers(ctx context.Context, organization string, repository string, number int, reviewers []string) error

// ListReviews is used to list all submitted reviews for a PR.
ListReviews(ctx context.Context, organization string, repository string, number int) (map[string]*github.Review, error)

// ListPullRequests returns a list of Pull Requests.
ListPullRequests(ctx context.Context, organization string, repository string, state string) ([]github.PullRequest, error)

// ListFiles is used to list all the files within a PR.
ListFiles(ctx context.Context, organization string, repository string, number int) ([]string, error)

// AddLabels will add labels to an Issue or Pull Request.
AddLabels(ctx context.Context, organization string, repository string, number int, labels []string) error

// ListWorkflows lists all workflows within a repository.
ListWorkflows(ctx context.Context, organization string, repository string) ([]github.Workflow, error)

// ListWorkflowRuns is used to list all workflow runs for an ID.
ListWorkflowRuns(ctx context.Context, organization string, repository string, branch string, workflowID int64) ([]github.Run, error)

// DeleteWorkflowRun is used to delete a workflow run.
DeleteWorkflowRun(ctx context.Context, organization string, repository string, runID int64) error
}

// Config contains configuration for the bot.
type Config struct {
// GitHub is a GitHub client.
GitHub github.Client
GitHub Client

// Environment holds information about the workflow run event.
Environment *env.Environment
Expand Down
32 changes: 16 additions & 16 deletions .github/workflows/robot/internal/bot/label.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,26 +87,26 @@ func deduplicate(s []string) []string {
}

var out []string
for k, _ := range m {
for k := range m {
out = append(out, k)
}

return out
}

var prefixes map[string][]string = map[string][]string{
"bpf/": []string{"bpf"},
"docs/": []string{"documentation"},
"rfd/": []string{"rfd"},
"examples/chart": []string{"helm"},
"lib/bpf/": []string{"bpf"},
"lib/events": []string{"audit-log"},
"lib/kube": []string{"kubernetes"},
"lib/srv/desktop": []string{"desktop-access"},
"lib/srv/desktop/rdp": []string{"desktop-access", "rdp"},
"lib/srv/app/": []string{"application-access"},
"lib/srv/db": []string{"database-access"},
"lib/web/desktop.go": []string{"desktop-access"},
"tool/tctl/": []string{"tctl"},
"tool/tsh/": []string{"tsh"},
var prefixes = map[string][]string{
"bpf/": {"bpf"},
"docs/": {"documentation"},
"rfd/": {"rfd"},
"examples/chart": {"helm"},
"lib/bpf/": {"bpf"},
"lib/events": {"audit-log"},
"lib/kube": {"kubernetes"},
"lib/srv/desktop": {"desktop-access"},
"lib/srv/desktop/rdp": {"desktop-access", "rdp"},
"lib/srv/app/": {"application-access"},
"lib/srv/db": {"database-access"},
"lib/web/desktop.go": {"desktop-access"},
"tool/tctl/": {"tctl"},
"tool/tsh/": {"tsh"},
}
51 changes: 12 additions & 39 deletions .github/workflows/robot/internal/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,48 +31,21 @@ import (
"golang.org/x/oauth2"
)

// Client implements the GitHub API.
type Client interface {
// RequestReviewers is used to assign reviewers to a PR.
RequestReviewers(ctx context.Context, organization string, repository string, number int, reviewers []string) error

// ListReviews is used to list all submitted reviews for a PR.
ListReviews(ctx context.Context, organization string, repository string, number int) (map[string]*Review, error)

// ListPullRequests returns a list of Pull Requests.
ListPullRequests(ctx context.Context, organization string, repository string, state string) ([]PullRequest, error)

// ListFiles is used to list all the files within a PR.
ListFiles(ctx context.Context, organization string, repository string, number int) ([]string, error)

// AddLabels will add labels to an Issue or Pull Request.
AddLabels(ctx context.Context, organization string, repository string, number int, labels []string) error

// ListWorkflows lists all workflows within a repository.
ListWorkflows(ctx context.Context, organization string, repository string) ([]Workflow, error)

// ListWorkflowRuns is used to list all workflow runs for an ID.
ListWorkflowRuns(ctx context.Context, organization string, repository string, branch string, workflowID int64) ([]Run, error)

// DeleteWorkflowRun is used to delete a workflow run.
DeleteWorkflowRun(ctx context.Context, organization string, repository string, runID int64) error
}

type client struct {
type Client struct {
client *go_github.Client
}

// New returns a new GitHub client.
func New(ctx context.Context, token string) (*client, error) {
// New returns a new GitHub Client.
func New(ctx context.Context, token string) (*Client, error) {
ts := oauth2.StaticTokenSource(
&oauth2.Token{AccessToken: token},
)
return &client{
return &Client{
client: go_github.NewClient(oauth2.NewClient(ctx, ts)),
}, nil
}

func (c *client) RequestReviewers(ctx context.Context, organization string, repository string, number int, reviewers []string) error {
func (c *Client) RequestReviewers(ctx context.Context, organization string, repository string, number int, reviewers []string) error {
_, _, err := c.client.PullRequests.RequestReviewers(ctx,
organization,
repository,
Expand All @@ -96,7 +69,7 @@ type Review struct {
SubmittedAt time.Time
}

func (c *client) ListReviews(ctx context.Context, organization string, repository string, number int) (map[string]*Review, error) {
func (c *Client) ListReviews(ctx context.Context, organization string, repository string, number int) (map[string]*Review, error) {
reviews := map[string]*Review{}

opt := &go_github.ListOptions{
Expand Down Expand Up @@ -152,7 +125,7 @@ type PullRequest struct {
Fork bool
}

func (c *client) ListPullRequests(ctx context.Context, organization string, repository string, state string) ([]PullRequest, error) {
func (c *Client) ListPullRequests(ctx context.Context, organization string, repository string, state string) ([]PullRequest, error) {
var pulls []PullRequest

opt := &go_github.PullRequestListOptions{
Expand Down Expand Up @@ -188,7 +161,7 @@ func (c *client) ListPullRequests(ctx context.Context, organization string, repo
return pulls, nil
}

func (c *client) ListFiles(ctx context.Context, organization string, repository string, number int) ([]string, error) {
func (c *Client) ListFiles(ctx context.Context, organization string, repository string, number int) ([]string, error) {
var files []string

opt := &go_github.ListOptions{
Expand Down Expand Up @@ -219,7 +192,7 @@ func (c *client) ListFiles(ctx context.Context, organization string, repository
}

// AddLabels will add labels to an Issue or Pull Request.
func (c *client) AddLabels(ctx context.Context, organization string, repository string, number int, labels []string) error {
func (c *Client) AddLabels(ctx context.Context, organization string, repository string, number int, labels []string) error {
_, _, err := c.client.Issues.AddLabelsToIssue(ctx,
organization,
repository,
Expand All @@ -242,7 +215,7 @@ type Workflow struct {
Path string
}

func (c *client) ListWorkflows(ctx context.Context, organization string, repository string) ([]Workflow, error) {
func (c *Client) ListWorkflows(ctx context.Context, organization string, repository string) ([]Workflow, error) {
var workflows []Workflow

opt := &go_github.ListOptions{
Expand Down Expand Up @@ -287,7 +260,7 @@ type Run struct {
CreatedAt time.Time
}

func (c *client) ListWorkflowRuns(ctx context.Context, organization string, repository string, branch string, workflowID int64) ([]Run, error) {
func (c *Client) ListWorkflowRuns(ctx context.Context, organization string, repository string, branch string, workflowID int64) ([]Run, error) {
var runs []Run

opt := &go_github.ListWorkflowRunsOptions{
Expand Down Expand Up @@ -330,7 +303,7 @@ func (c *client) ListWorkflowRuns(ctx context.Context, organization string, repo
// DeleteWorkflowRun is directly implemented because it is missing from go-github.
//
// https://docs.github.com/en/rest/reference/actions#delete-a-workflow-run
func (c *client) DeleteWorkflowRun(ctx context.Context, organization string, repository string, runID int64) error {
func (c *Client) DeleteWorkflowRun(ctx context.Context, organization string, repository string, runID int64) error {
url := url.URL{
Scheme: "https",
Host: "api.github.com",
Expand Down
33 changes: 32 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,14 @@ $(RENDER_TESTS): $(wildcard ./build.assets/tooling/cmd/render-tests/*.go)
.PHONY: test
test: test-sh test-api test-go

# Runs bot Go tests.
#
.PHONY: test-bot
test-bot:
test-bot: FLAGS ?= '-race'
test-bot:
cd .github/workflows/robot && go test $(FLAGS) ./...

#
# Runs all Go tests except integration, called by CI/CD.
# Chaos tests have high concurrency, run without race detector and have TestChaos prefix.
Expand Down Expand Up @@ -532,7 +540,10 @@ integration-root: $(RENDER_TESTS)
# changes (or last commit).
#
.PHONY: lint
lint: lint-sh lint-helm lint-api lint-go lint-license lint-rdp
lint: lint-sh lint-helm lint-api lint-go lint-license lint-rdp lint-tools

.PHONY: lint-tools
lint-tools: lint-version-check lint-bot lint-ci-scripts lint-backport

.PHONY: lint-rdp
lint-rdp:
Expand All @@ -545,6 +556,26 @@ lint-go: GO_LINT_FLAGS ?=
lint-go:
golangci-lint run -c .golangci.yml $(GO_LINT_FLAGS)

.PHONY: lint-version-check
lint-version-check: GO_LINT_FLAGS ?=
lint-version-check:
cd build.assets/version-check && golangci-lint run -c ../../.golangci.yml $(GO_LINT_FLAGS)

.PHONY: lint-backport
lint-backport: GO_LINT_FLAGS ?=
lint-backport:
cd assets/backport && golangci-lint run -c ../../.golangci.yml $(GO_LINT_FLAGS)

.PHONY: lint-bot
lint-bot: GO_LINT_FLAGS ?=
lint-bot:
cd .github/workflows/robot && golangci-lint run -c ../../../.golangci.yml $(GO_LINT_FLAGS)

.PHONY: lint-ci-scripts
lint-ci-scripts: GO_LINT_FLAGS ?=
lint-ci-scripts:
cd .cloudbuild/scripts/ && golangci-lint run -c ../../.golangci.yml $(GO_LINT_FLAGS)

# api is no longer part of the teleport package, so golangci-lint skips it by default
.PHONY: lint-api
lint-api: GO_LINT_API_FLAGS ?=
Expand Down
17 changes: 8 additions & 9 deletions assets/backport/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,21 @@ func main() {
fmt.Println("Backporting complete.")
}

// Config is used to unmarshal the Github
// Config is used to unmarshal the Github
// CLI config.
type Config struct {
// Host is the host name of the
// server.
// Host is the host name of the
// server.
Host Host `yaml:"github.com"`
}

type Host struct {
// Token is Github token.
// Token is Github token.
Token string `yaml:"oauth_token"`
}

// githubConfigPath is the default config path
// (relative to user's home directory) for the
// githubConfigPath is the default config path
// (relative to user's home directory) for the
// Github CLI tool.
const githubConfigPath = ".config/gh/hosts.yml"

Expand All @@ -105,8 +105,7 @@ func getGithubToken() (string, error) {
return "", trace.Wrap(err)
}

var config *Config = new(Config)

config := new(Config)
if err = yaml.Unmarshal(yamlFile, config); err != nil {
return "", trace.Wrap(err)
}
Expand Down Expand Up @@ -146,7 +145,7 @@ func parseBranches(branchesInput string) ([]string, error) {
branches := strings.Split(branchesInput, ",")
for _, branch := range branches {
if branch == "" {
return nil, trace.BadParameter("recieved an empty branch name.")
return nil, trace.BadParameter("received an empty branch name.")
}
backportBranches = append(backportBranches, strings.TrimSpace(branch))
}
Expand Down
2 changes: 1 addition & 1 deletion assets/backport/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand Down
6 changes: 3 additions & 3 deletions build.assets/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ ENV GOPATH="/go" \
# Install addlicense
RUN go install github.com/google/[email protected]

# Install meta-linter.
RUN (curl -L https://github.com/golangci/golangci-lint/releases/download/v1.38.0/golangci-lint-1.38.0-$(go env GOOS)-$(go env GOARCH).tar.gz | tar -xz && \
cp golangci-lint-1.38.0-$(go env GOOS)-$(go env GOARCH)/golangci-lint /bin/ && \
# Install golangci-lint.
RUN (curl -L https://github.com/golangci/golangci-lint/releases/download/v1.44.0/golangci-lint-1.44.0-$(go env GOOS)-$(go env GOARCH).tar.gz | tar -xz && \
cp golangci-lint-1.44.0-$(go env GOOS)-$(go env GOARCH)/golangci-lint /bin/ && \
rm -r golangci-lint*)

# Install helm.
Expand Down
2 changes: 1 addition & 1 deletion lib/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -2902,7 +2902,7 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error {

// Register web proxy server
var webServer *http.Server
var webHandler *web.WebAPIHandler
var webHandler *web.APIHandler
if !process.Config.Proxy.DisableWebService {
var fs http.FileSystem
if !process.Config.Proxy.DisableWebInterface {
Expand Down
10 changes: 5 additions & 5 deletions lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ type Config struct {
ProxySettings proxySettingsGetter
}

type WebAPIHandler struct {
type APIHandler struct {
handler *Handler

// appHandler is a http.Handler to forward requests to applications.
Expand All @@ -198,7 +198,7 @@ type WebAPIHandler struct {

// Check if this request should be forwarded to an application handler to
// be handled by the UI and handle the request appropriately.
func (h *WebAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
func (h *APIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// If the request is either to the fragment authentication endpoint or if the
// request is already authenticated (has a session cookie), forward to
// application handlers. If the request is unauthenticated and requesting a
Expand All @@ -215,12 +215,12 @@ func (h *WebAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
h.handler.ServeHTTP(w, r)
}

func (h *WebAPIHandler) Close() error {
func (h *APIHandler) Close() error {
return h.handler.Close()
}

// NewHandler returns a new instance of web proxy handler
func NewHandler(cfg Config, opts ...HandlerOption) (*WebAPIHandler, error) {
func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) {
const apiPrefix = "/" + teleport.WebAPIVersion
h := &Handler{
cfg: cfg,
Expand Down Expand Up @@ -518,7 +518,7 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*WebAPIHandler, error) {
return nil, trace.Wrap(err)
}

return &WebAPIHandler{
return &APIHandler{
handler: h,
appHandler: appHandler,
}, nil
Expand Down
2 changes: 1 addition & 1 deletion lib/web/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3355,7 +3355,7 @@ type proxy struct {
revTun reversetunnel.Server
node *regular.Server
proxy *regular.Server
handler *WebAPIHandler
handler *APIHandler
web *httptest.Server
webURL url.URL
}
Expand Down

0 comments on commit 896261a

Please sign in to comment.