Skip to content

Commit

Permalink
[ACIX-424] Add codeowners linter for .gitlab files (#29873)
Browse files Browse the repository at this point in the history
Co-authored-by: Nicolas Schweitzer <nicolas.schweitzer@datadoghq.com>
Co-authored-by: Kevin Fairise <132568982+KevinFairise2@users.noreply.github.com>
Co-authored-by: Alexandre Menasria <47357713+amenasria@users.noreply.github.com>
  • Loading branch information
4 people authored Oct 9, 2024
1 parent 0ea8e91 commit 1f3c2e8
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 4 deletions.
43 changes: 42 additions & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,48 @@
# Files containing job contents are owned by teams in charge of the jobs + agent-devx-infra or agent-delivery
# Files that only describe structure (eg. includes, rules) are owned by agent-devx-infra

/.gitlab/ @DataDog/agent-devx-infra
/.gitlab/.ci-linters.yml @DataDog/agent-devx-infra
/.gitlab/.pre/* @DataDog/agent-devx-infra
/.gitlab/check_deploy/* @DataDog/agent-delivery
/.gitlab/check_merge/* @DataDog/agent-devx-infra
/.gitlab/deploy*/* @DataDog/agent-delivery
/.gitlab/deps_fetch/* @DataDog/agent-devx-infra
/.gitlab/e2e/* @DataDog/agent-devx-infra @DataDog/agent-devx-loops
/.gitlab/e2e_install_packages/* @DataDog/agent-delivery
/.gitlab/e2e_pre_test/* @DataDog/agent-devx-infra @DataDog/agent-devx-loops
/.gitlab/junit_upload/* @DataDog/agent-devx-infra
/.gitlab/kernel_matrix_testing/* @DataDog/agent-devx-infra @DataDog/ebpf-platform
/.gitlab/lint/* @DataDog/agent-devx-infra
/.gitlab/maintenance_jobs/* @DataDog/agent-devx-infra @DataDog/agent-devx-loops
/.gitlab/notify/* @DataDog/agent-devx-infra
/.gitlab/pkg_metrics/* @DataDog/agent-devx-infra
/.gitlab/post_rc_build/* @DataDog/agent-devx-infra
/.gitlab/setup/* @DataDog/agent-devx-infra
/.gitlab/trigger_release/* @DataDog/agent-devx-infra

/.gitlab/binary_build/cws_instrumentation.yml @DataDog/agent-devx-infra @DataDog/agent-security
/.gitlab/binary_build/include.yml @DataDog/agent-devx-infra
/.gitlab/binary_build/linux.yml @DataDog/agent-devx-infra @DataDog/agent-delivery
/.gitlab/functional_test/include.yml @DataDog/agent-devx-infra
/.gitlab/functional_test_cleanup/functional_test_cleanup.yml @DataDog/agent-devx-infra @DataDog/agent-security
/.gitlab/install_script_testing/install_script_testing.yml @DataDog/agent-delivery
/.gitlab/integration_test/dogstatsd.yml @DataDog/agent-devx-infra @DataDog/agent-metrics-logs
/.gitlab/integration_test/include.yml @DataDog/agent-devx-infra
/.gitlab/integration_test/linux.yml @DataDog/agent-devx-infra
/.gitlab/integration_test/otel.yml @DataDog/agent-devx-infra @DataDog/opentelemetry
/.gitlab/internal_image_deploy/internal_image_deploy.yml @DataDog/agent-delivery
/.gitlab/internal_kubernetes_deploy/include.yml @DataDog/agent-devx-infra
/.gitlab/internal_kubernetes_deploy/internal_kubernetes_deploy.yml @DataDog/agent-delivery
/.gitlab/internal_kubernetes_deploy/rc_kubernetes_deploy.yml @DataDog/agent-delivery
/.gitlab/package_deps_build/package_deps_build.yml @DataDog/agent-devx-infra @DataDog/ebpf-platform
/.gitlab/powershell_script_signing/powershell_script_signing.yml @DataDog/agent-delivery @DataDog/windows-agent
/.gitlab/source_test/golang_deps_diff.yml @DataDog/agent-devx-infra @DataDog/agent-devx-loops
/.gitlab/source_test/include.yml @DataDog/agent-devx-infra
/.gitlab/source_test/linux.yml @DataDog/agent-devx-infra @DataDog/agent-devx-loops
/.gitlab/source_test/macos.yml @DataDog/agent-devx-infra @DataDog/agent-devx-loops
/.gitlab/source_test/notify.yml @DataDog/agent-devx-infra @DataDog/agent-devx-loops
/.gitlab/source_test/slack.yml @DataDog/agent-devx-infra @DataDog/agent-devx-loops
/.gitlab/source_test/tooling_unit_tests.yml @DataDog/agent-devx-infra @DataDog/agent-devx-loops

/.gitlab/binary_build/cluster_agent_cloudfoundry.yml @DataDog/platform-integrations @DataDog/agent-delivery
/.gitlab/binary_build/cluster_agent.yml @DataDog/container-integrations @DataDog/agent-delivery
Expand Down
10 changes: 10 additions & 0 deletions .gitlab/.pre/gitlab_configuration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,13 @@ lint_gitlab_ci:
script:
- inv -e linter.gitlab-ci-jobs-needs-rules --config-file artifacts/after.gitlab-ci.yml
- inv -e linter.gitlab-ci-jobs-owners --config-file artifacts/after.gitlab-ci.yml

lint_gitlab_ci_jobs_codeowners:
stage: .pre
image: 486234852809.dkr.ecr.us-east-1.amazonaws.com/ci/datadog-agent-buildimages/deb_arm64$DATADOG_AGENT_BUILDIMAGES_SUFFIX:$DATADOG_AGENT_BUILDIMAGES
tags: ["arch:arm64"]
needs: []
rules:
- !reference [.on_gitlab_changes]
script:
- inv -e linter.gitlab-ci-jobs-codeowners
49 changes: 47 additions & 2 deletions tasks/libs/common/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,54 @@ def get_staged_files(ctx, commit="HEAD", include_deleted_files=False) -> Iterabl
yield file


def get_file_modifications(
ctx, base_branch=None, added=False, modified=False, removed=False, only_names=False, no_renames=False
) -> list[tuple[str, str]]:
"""Gets file status changes for the current branch compared to the base branch.
If no filter is provided, will return all the files.
Args:
added: Include added files
modified: Include modified files
removed: Include removed files
only_names: Return only the file names without the status
no_renames: Do not include renamed files
Returns:
A list of (status, filename)
"""

from tasks.libs.releasing.json import _get_release_json_value

base_branch = base_branch or _get_release_json_value('base_branch')

last_main_commit = ctx.run(f"git merge-base HEAD origin/{base_branch}", hide=True).stdout.strip()

flags = '--no-renames' if no_renames else ''

modifications = [
line.split()
for line in ctx.run(f"git diff --name-status {flags} {last_main_commit}", hide=True).stdout.splitlines()
]

if added or modified or removed:
modifications = [
(status, file)
for status, file in modifications
if (added and status == "A") or (modified and status in "MCRT") or (removed and status == "D")
]

if only_names:
modifications = [file for _, file in modifications]

return modifications


def get_modified_files(ctx, base_branch="main") -> list[str]:
last_main_commit = ctx.run(f"git merge-base HEAD origin/{base_branch}", hide=True).stdout
return ctx.run(f"git diff --name-only --no-renames {last_main_commit}", hide=True).stdout.splitlines()
return get_file_modifications(
ctx, base_branch=base_branch, added=True, modified=True, only_names=True, no_renames=True
)


def get_current_branch(ctx) -> str:
Expand Down
44 changes: 43 additions & 1 deletion tasks/linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from glob import glob

import yaml
from codeowners import CodeOwners
from invoke import Exit, task

from tasks.build_tags import compute_build_tags_for_flavor
Expand All @@ -32,7 +33,7 @@
from tasks.libs.common.check_tools_version import check_tools_version
from tasks.libs.common.color import Color, color_message
from tasks.libs.common.constants import DEFAULT_BRANCH, GITHUB_REPO_NAME
from tasks.libs.common.git import get_staged_files
from tasks.libs.common.git import get_file_modifications, get_staged_files
from tasks.libs.common.utils import gitlab_section, is_pr_context, running_in_ci
from tasks.libs.owners.parsing import read_owners
from tasks.libs.types.copyright import CopyrightLinter, LintFailure
Expand Down Expand Up @@ -793,3 +794,44 @@ def gitlab_ci_jobs_owners(_, diff_file=None, config_file=None, path_jobowners='.
)
else:
print(f'{color_message("Success", Color.GREEN)}: All jobs have owners defined in {path_jobowners}')


def _gitlab_ci_jobs_codeowners_lint(path_codeowners, modified_yml_files, gitlab_owners):
error_files = []
for path in modified_yml_files:
teams = [team for kind, team in gitlab_owners.of(path) if kind == 'TEAM']
if not teams:
error_files.append(path)

if error_files:
error_files = '\n'.join(f'- {path}' for path in sorted(error_files))

raise Exit(
f"{color_message('Error', Color.RED)}: These files should have specific CODEOWNERS rules within {path_codeowners} starting with '/.gitlab/<stage_name>'):\n{error_files}"
)
else:
print(f'{color_message("Success", Color.GREEN)}: All files have CODEOWNERS rules within {path_codeowners}')


@task
def gitlab_ci_jobs_codeowners(ctx, path_codeowners='.github/CODEOWNERS', all_files=False):
"""Verifies that added / modified job files are defined within CODEOWNERS."""

if all_files:
modified_yml_files = glob('.gitlab/**/*.yml', recursive=True)
else:
modified_yml_files = get_file_modifications(ctx, added=True, modified=True, only_names=True)
modified_yml_files = [path for path in modified_yml_files if path.endswith('.yml')]

if not modified_yml_files:
print(f'{color_message("Info", Color.BLUE)}: No added / modified job files, skipping lint')
return

with open(path_codeowners) as f:
parsed_owners = f.readlines()

# Keep only gitlab related lines to avoid defaults
parsed_owners = [line for line in parsed_owners if '/.gitlab/' in line]
gitlab_owners = CodeOwners('\n'.join(parsed_owners))

_gitlab_ci_jobs_codeowners_lint(path_codeowners, modified_yml_files, gitlab_owners)
40 changes: 40 additions & 0 deletions tasks/unit_tests/linter_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import unittest
from unittest.mock import MagicMock, patch

from codeowners import CodeOwners
from invoke import Exit, MockContext

import tasks.linter as linter
Expand Down Expand Up @@ -123,3 +124,42 @@ def test_error(self):
):
with self.assertRaises(Exit):
linter.gitlab_ci_jobs_needs_rules(MockContext())


class TestGitlabCIJobsCodeowners(unittest.TestCase):
def test_no_file(self):
codeowners = """
/somefile @DataDog/the-best-team
/.* @DataDog/another-best-team
"""

linter._gitlab_ci_jobs_codeowners_lint('/path/to/codeowners', [], CodeOwners(codeowners))

def test_one_file(self):
codeowners = """
/somefile @DataDog/the-best-team
/.* @DataDog/another-best-team
"""

linter._gitlab_ci_jobs_codeowners_lint('/path/to/codeowners', ['somefile'], CodeOwners(codeowners))

def test_multiple_files(self):
codeowners = """
/somefile @DataDog/the-best-team
/.* @DataDog/another-best-team
"""

linter._gitlab_ci_jobs_codeowners_lint(
'/path/to/codeowners', ['somefile', '.gitlab-ci.yml'], CodeOwners(codeowners)
)

def test_error(self):
codeowners = """
/somefile @DataDog/the-best-team
/.* @DataDog/another-best-team
"""

with self.assertRaises(Exit):
linter._gitlab_ci_jobs_codeowners_lint(
'/path/to/codeowners', ['becareful', '.gitlab-ci.yml'], CodeOwners(codeowners)
)

0 comments on commit 1f3c2e8

Please sign in to comment.