From 23236c12fae5de5d45b40959718b948beb95720f Mon Sep 17 00:00:00 2001 From: Mohammed Naser Date: Tue, 31 Mar 2020 19:17:14 -0400 Subject: [PATCH] golangci-lint: add job This patch adds a roles and jobs to run golangci-lint against a a Golang project. It's a very popular tool for linting go code. It also adds a simple framework which allows us to create dynamic tests for file comments by defining a simple YAML file. Change-Id: Ic8358541adaf7c3279383f0279cd3da7b446a6e0 --- doc/source/go-jobs.rst | 1 + doc/source/go-roles.rst | 2 + playbooks/golangci-lint/pre.yaml | 5 ++ playbooks/golangci-lint/run.yaml | 4 + roles/ensure-golangci-lint/README.rst | 7 ++ roles/ensure-golangci-lint/defaults/main.yaml | 5 ++ roles/ensure-golangci-lint/tasks/main.yaml | 9 ++ roles/golangci-lint/README.rst | 12 +++ roles/golangci-lint/__init__.py | 0 roles/golangci-lint/defaults/main.yaml | 4 + roles/golangci-lint/library/__init__.py | 0 .../library/golangci_lint_parse_output.py | 87 +++++++++++++++++++ .../library/test-cases/failed_build.yaml | 11 +++ .../library/test-cases/multiple_lines.yaml | 11 +++ .../library/test-cases/no_output.yaml | 5 ++ .../library/test-cases/single_line.yaml | 28 ++++++ .../test_golangci_lint_parse_output.py | 33 +++++++ roles/golangci-lint/tasks/main.yaml | 27 ++++++ tests/__init__.py | 35 ++++++++ zuul.d/go-jobs.yaml | 8 ++ 20 files changed, 294 insertions(+) create mode 100644 playbooks/golangci-lint/pre.yaml create mode 100644 playbooks/golangci-lint/run.yaml create mode 100644 roles/ensure-golangci-lint/README.rst create mode 100644 roles/ensure-golangci-lint/defaults/main.yaml create mode 100644 roles/ensure-golangci-lint/tasks/main.yaml create mode 100644 roles/golangci-lint/README.rst create mode 100644 roles/golangci-lint/__init__.py create mode 100644 roles/golangci-lint/defaults/main.yaml create mode 100644 roles/golangci-lint/library/__init__.py create mode 100644 roles/golangci-lint/library/golangci_lint_parse_output.py create mode 100644 roles/golangci-lint/library/test-cases/failed_build.yaml create mode 100644 roles/golangci-lint/library/test-cases/multiple_lines.yaml create mode 100644 roles/golangci-lint/library/test-cases/no_output.yaml create mode 100644 roles/golangci-lint/library/test-cases/single_line.yaml create mode 100644 roles/golangci-lint/library/test_golangci_lint_parse_output.py create mode 100644 roles/golangci-lint/tasks/main.yaml diff --git a/doc/source/go-jobs.rst b/doc/source/go-jobs.rst index f6225713a..a6c64789a 100644 --- a/doc/source/go-jobs.rst +++ b/doc/source/go-jobs.rst @@ -3,3 +3,4 @@ Go Jobs .. zuul:autojob:: golang-go .. zuul:autojob:: golang-go-test +.. zuul:autojob:: golangci-lint diff --git a/doc/source/go-roles.rst b/doc/source/go-roles.rst index c6ed2d7ff..53ba1fdd3 100644 --- a/doc/source/go-roles.rst +++ b/doc/source/go-roles.rst @@ -1,5 +1,7 @@ Go Roles ======== +.. zuul:autorole:: ensure-golangci-lint .. zuul:autorole:: install-go .. zuul:autorole:: go +.. zuul:autorole:: golangci-lint diff --git a/playbooks/golangci-lint/pre.yaml b/playbooks/golangci-lint/pre.yaml new file mode 100644 index 000000000..9c2366e3a --- /dev/null +++ b/playbooks/golangci-lint/pre.yaml @@ -0,0 +1,5 @@ +--- +- hosts: all + roles: + - install-go + - ensure-golangci-lint diff --git a/playbooks/golangci-lint/run.yaml b/playbooks/golangci-lint/run.yaml new file mode 100644 index 000000000..f40923b57 --- /dev/null +++ b/playbooks/golangci-lint/run.yaml @@ -0,0 +1,4 @@ +--- +- hosts: all + roles: + - golangci-lint diff --git a/roles/ensure-golangci-lint/README.rst b/roles/ensure-golangci-lint/README.rst new file mode 100644 index 000000000..f55eef6ec --- /dev/null +++ b/roles/ensure-golangci-lint/README.rst @@ -0,0 +1,7 @@ +Ensure golangci-lint is installed + +**Role Variables** + +.. zuul:rolevar:: golangci_lint_version + + Version of golangci-lint to install diff --git a/roles/ensure-golangci-lint/defaults/main.yaml b/roles/ensure-golangci-lint/defaults/main.yaml new file mode 100644 index 000000000..4a4459136 --- /dev/null +++ b/roles/ensure-golangci-lint/defaults/main.yaml @@ -0,0 +1,5 @@ +--- +# NOTE(mnaser): We are pinning to 1.23.8 due to the fact that at the time of +# writing this role, 1.24.0 has memory issues +# https://github.com/golangci/golangci-lint/issues/994 +golangci_lint_version: 1.23.8 diff --git a/roles/ensure-golangci-lint/tasks/main.yaml b/roles/ensure-golangci-lint/tasks/main.yaml new file mode 100644 index 000000000..379029b5e --- /dev/null +++ b/roles/ensure-golangci-lint/tasks/main.yaml @@ -0,0 +1,9 @@ +--- +- name: Install golangci-lint + become: true + unarchive: + remote_src: true + src: "https://github.com/golangci/golangci-lint/releases/download/v{{ golangci_lint_version }}/golangci-lint-{{ golangci_lint_version }}-linux-amd64.tar.gz" + dest: /usr/local/bin + extra_opts: + - --strip-components=1 diff --git a/roles/golangci-lint/README.rst b/roles/golangci-lint/README.rst new file mode 100644 index 000000000..f6394584a --- /dev/null +++ b/roles/golangci-lint/README.rst @@ -0,0 +1,12 @@ +Run golangci-lint + +**Role Variables** + +.. zuul:rolevar:: zuul_work_dir + :default: {{ zuul.project.src_dir }} + + The location of the main working directory of the job. + +.. zuul:rolevar:: golangci_lint_options + + Arguments passed to golangci-lint diff --git a/roles/golangci-lint/__init__.py b/roles/golangci-lint/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/roles/golangci-lint/defaults/main.yaml b/roles/golangci-lint/defaults/main.yaml new file mode 100644 index 000000000..49e9852bc --- /dev/null +++ b/roles/golangci-lint/defaults/main.yaml @@ -0,0 +1,4 @@ +zuul_work_dir: "{{ zuul.project.src_dir }}" +golangci_lint_options: "" +go_install_dir: "/usr/local" +go_bin_path: "{{ go_install_dir }}/go/bin" diff --git a/roles/golangci-lint/library/__init__.py b/roles/golangci-lint/library/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/roles/golangci-lint/library/golangci_lint_parse_output.py b/roles/golangci-lint/library/golangci_lint_parse_output.py new file mode 100644 index 000000000..c1209630b --- /dev/null +++ b/roles/golangci-lint/library/golangci_lint_parse_output.py @@ -0,0 +1,87 @@ +#!/usr/bin/python + +# Copyright 2020 VEXXHOST, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); 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. See the +# License for the specific language governing permissions and limitations +# under the License. +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +DOCUMENTATION = ''' +--- +module: golangci_lint_parse_output +short_description: Parse the output of golangci-lint and return comments +author: Mohammed Naser (@mnaser) +description: + - Parse the output of golangci-lint and return content for inline comments. +requirements: + - "python >= 3.5" +options: + workdir: + description: + - Path for the project to strip for comments + required: true + type: str + output: + description: + - Output from the golangci-lint command run + required: true + type: str +''' + +import re + +from ansible.module_utils.basic import AnsibleModule + +BUILD_RE = re.compile(r'^.*\[(.*)\]"$') +RE = re.compile(r"^(.*):(\d+):\d+: (.*)$") + + +def parse_output(output, workdir): + comments = {} + for line in output.split('\n'): + # If we have a build failure, we need to match that first and extract + # the error message. We'll also need to remove 'workdir' as it + # contains the full path. + m = BUILD_RE.match(line) + if m: + line = re.sub(r'\\(.)', r'\1', m.group(1)) + # We find everything up until workdir, strip the length of workdir + # and then remove one extra character to remove leading slash. + index = line.find(workdir) + len(workdir) + 1 + line = line[index:].replace(workdir, '') + m = RE.match(line) + if m: + file_path = m.group(1) + start_line = m.group(2) + message = m.group(3) + + comments.setdefault(file_path, []) + comments[file_path].append(dict(line=int(start_line), + message=message)) + return comments + + +def main(): + module = AnsibleModule( + argument_spec=dict( + output=dict(required=True, type='str', no_log=True), + workdir=dict(required=True, str='str'), + ) + ) + + comments = parse_output(module.params['output'], module.params['workdir']) + module.exit_json(changed=False, comments=comments) + + +if __name__ == '__main__': + main() diff --git a/roles/golangci-lint/library/test-cases/failed_build.yaml b/roles/golangci-lint/library/test-cases/failed_build.yaml new file mode 100644 index 000000000..0f974bc96 --- /dev/null +++ b/roles/golangci-lint/library/test-cases/failed_build.yaml @@ -0,0 +1,11 @@ +--- +workdir: src/opendev.org/vexxhost/openstack-operator +output: | + level=warning msg="[runner] Can't run linter unused: buildssa: analysis skipped: errors in package: [/home/zuul/src/opendev.org/vexxhost/openstack-operator/builders/pod_metrics_endpoint.go:4:2: \"k8s.io/apimachinery/pkg/util/intstr\" imported but not used]" + level=warning msg="[runner] Can't run linter goanalysis_metalinter: S1009: failed prerequisites: inspect@opendev.org/vexxhost/openstack-operator/builders, isgenerated@opendev.org/vexxhost/openstack-operator/builders" + level=error msg="Running error: S1009: failed prerequisites: inspect@opendev.org/vexxhost/openstack-operator/builders, isgenerated@opendev.org/vexxhost/openstack-operator/builders" + +comments: + builders/pod_metrics_endpoint.go: + - line: 4 + message: '"k8s.io/apimachinery/pkg/util/intstr" imported but not used' diff --git a/roles/golangci-lint/library/test-cases/multiple_lines.yaml b/roles/golangci-lint/library/test-cases/multiple_lines.yaml new file mode 100644 index 000000000..914c79c72 --- /dev/null +++ b/roles/golangci-lint/library/test-cases/multiple_lines.yaml @@ -0,0 +1,11 @@ +--- +workdir: src/opendev.org/vexxhost/openstack-operator +output: | + builders/pod_metrics_endpoint.go:27:2: SA1019: pme.obj.TargetPort is deprecated: Use 'port' instead. (staticcheck) + pme.obj.TargetPort = &targetPort + ^ + +comments: + builders/pod_metrics_endpoint.go: + - line: 27 + message: "SA1019: pme.obj.TargetPort is deprecated: Use 'port' instead. (staticcheck)" diff --git a/roles/golangci-lint/library/test-cases/no_output.yaml b/roles/golangci-lint/library/test-cases/no_output.yaml new file mode 100644 index 000000000..d55548b43 --- /dev/null +++ b/roles/golangci-lint/library/test-cases/no_output.yaml @@ -0,0 +1,5 @@ +--- +workdir: src/opendev.org/vexxhost/openstack-operator +output: "" + +comments: {} diff --git a/roles/golangci-lint/library/test-cases/single_line.yaml b/roles/golangci-lint/library/test-cases/single_line.yaml new file mode 100644 index 000000000..5d426d0d6 --- /dev/null +++ b/roles/golangci-lint/library/test-cases/single_line.yaml @@ -0,0 +1,28 @@ +--- +workdir: src/opendev.org/vexxhost/openstack-operator +output: | + builders/pod_metrics_endpoint.go:27:2: SA1019: pme.obj.TargetPort is deprecated: Use 'port' instead. (staticcheck) + pme.obj.TargetPort = &targetPort + ^ + controllers/mcrouter_controller.go:133:15: S1039: unnecessary use of fmt.Sprintf (gosimple) + Name: fmt.Sprintf("mcrouter-podmonitor"), + ^ + controllers/mcrouter_controller.go:163:15: S1039: unnecessary use of fmt.Sprintf (gosimple) + Name: fmt.Sprintf("mcrouter-alertrule"), + ^ + controllers/memcached_controller.go:130:15: S1039: unnecessary use of fmt.Sprintf (gosimple) + Name: fmt.Sprintf("memcached-podmonitor"), + +comments: + builders/pod_metrics_endpoint.go: + - line: 27 + message: "SA1019: pme.obj.TargetPort is deprecated: Use 'port' instead. (staticcheck)" + controllers/mcrouter_controller.go: + - line: 133 + message: "S1039: unnecessary use of fmt.Sprintf (gosimple)" + - line: 163 + message: "S1039: unnecessary use of fmt.Sprintf (gosimple)" + controllers/memcached_controller.go: + - line: 130 + message: "S1039: unnecessary use of fmt.Sprintf (gosimple)" + diff --git a/roles/golangci-lint/library/test_golangci_lint_parse_output.py b/roles/golangci-lint/library/test_golangci_lint_parse_output.py new file mode 100644 index 000000000..65c44f1af --- /dev/null +++ b/roles/golangci-lint/library/test_golangci_lint_parse_output.py @@ -0,0 +1,33 @@ +# Copyright (C) 2020 VEXXHOST, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# 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. +# +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + +import testtools + +from tests import generate_dynamic_comments_tests +from .golangci_lint_parse_output import parse_output + +TESTS_DIR = os.path.join(os.path.dirname(__file__), + 'test-cases') + + +class TestGolangciLintParseOutput(testtools.TestCase): + pass + + +generate_dynamic_comments_tests(TestGolangciLintParseOutput, TESTS_DIR, + parse_output) diff --git a/roles/golangci-lint/tasks/main.yaml b/roles/golangci-lint/tasks/main.yaml new file mode 100644 index 000000000..7f90a4598 --- /dev/null +++ b/roles/golangci-lint/tasks/main.yaml @@ -0,0 +1,27 @@ +- name: Run golangci-lint + command: "golangci-lint run {{ golangci_lint_options }}" + args: + chdir: "{{ zuul_work_dir }}" + environment: + PATH: "{{ ansible_env.PATH }}:{{ go_bin_path }}" + ignore_errors: true + register: _golangci_lint + +- name: Look for output + golangci_lint_parse_output: + workdir: '{{ zuul_work_dir }}' + output: '{{ _golangci_lint.stdout }}' + register: _golangci_lint_parse_output + +- name: Return file comments to Zuul + when: _golangci_lint_parse_output.comments + delegate_to: localhost + zuul_return: + data: + zuul: + file_comments: '{{ _golangci_lint_parse_output.comments }}' + +- name: Return golangci-lint status + fail: + msg: 'golangci-lint exited with return code {{ _golangci_lint.rc }}' + when: _golangci_lint.rc != 0 diff --git a/tests/__init__.py b/tests/__init__.py index e69de29bb..1e7c6a2fb 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -0,0 +1,35 @@ +# Copyright (C) 2020 VEXXHOST, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# 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. +# +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + +import yaml + + +def generate_dynamic_comments_tests(cls, test_path, func): + def _create_test_using_file(name): + def test(self): + path = "%s/%s" % (test_path, name) + with open(path) as fd: + data = yaml.load(fd, Loader=yaml.FullLoader) + comments = func(data['output'], data['workdir']) + self.assertEqual(data['comments'], comments) + return test + + for t in os.listdir(test_path): + test = _create_test_using_file(t) + test.__name__ = "test_%s" % t.split('.')[0] + setattr(cls, test.__name__, test) diff --git a/zuul.d/go-jobs.yaml b/zuul.d/go-jobs.yaml index d43ca4fa0..b5945f666 100644 --- a/zuul.d/go-jobs.yaml +++ b/zuul.d/go-jobs.yaml @@ -90,3 +90,11 @@ Path to operate in. vars: go_command: test + +- job: + name: golangci-lint + parent: unittests + description: | + Run golangci-lint on a Go project + pre-run: playbooks/golangci-lint/pre.yaml + run: playbooks/golangci-lint/run.yaml