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.

Sign up for GitHub

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

Run copyright after running deep-copy as part of the Makefile/CI #18741

Merged
merged 7 commits into from
Sep 11, 2023
Merged

Run copyright after running deep-copy as part of the Makefile/CI #18741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5913e30
execute copyright headers after performing deep-copy generation.
dhiaayachi Sep 8, 2023
88735fd
Merge branch 'main' into dhia/NET-5262-deep-copy-copyright
dhiaayachi Sep 11, 2023
3b421ee
fix copyright install
dhiaayachi Sep 11, 2023
4f363f2
Apply suggestions from code review
dhiaayachi Sep 11, 2023
7de13c6
Apply suggestions from code review
dhiaayachi Sep 11, 2023
e08b5cb
rename steps to match codegen naming
dhiaayachi Sep 11, 2023
8c65311
remove copywrite install category
dhiaayachi Sep 11, 2023
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
48 changes: 23 additions & 25 deletions .github/workflows/go-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,29 +96,28 @@ jobs:
- name: Notify Slack
if: ${{ failure() }}
run: .github/scripts/notify_slack.sh
# Temporarily changing until the situation with license headers in the generated code can be worked out
# check-generated-deep-copy:
# needs:
# - setup
# runs-on: ${{ fromJSON(needs.setup.outputs.compute-large) }}
# steps:
# - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
# # NOTE: This step is specifically needed for ENT. It allows us to access the required private HashiCorp repos.
# - name: Setup Git
# if: ${{ endsWith(github.repository, '-enterprise') }}
# run: git config --global url."https://${{ secrets.ELEVATED_GITHUB_TOKEN }}:@github.com".insteadOf "https://github.com"
# - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1
# with:
# go-version-file: 'go.mod'
# - run: make --always-make deep-copy
# - run: |
# if ! git diff --exit-code; then
# echo "Generated code was not updated correctly"
# exit 1
# fi
# - name: Notify Slack
# if: ${{ failure() }}
# run: .github/scripts/notify_slack.sh
check-codegen:
Copy link
Collaborator

@analogue analogue Sep 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Since this is now a general "make sure copywrite headers are correct everywhere" (by using the configuration in .copwrite.hcl), does it make sense to rename to check-copywrite since it will complain about files that are not related to code generation?

Copy link
Collaborator Author

@dhiaayachi dhiaayachi Sep 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It actually does both checking that copywrite headers are inserted and deep-copy generated files are up to date.
I was thinking of separating both but it make it hard, as we will never be able to check the end state which is deep-copy file are generated and all files have the headers (including the deep-copy generated ones)

This is why I renamed it to codegen, as I'm counting BSL header generation as part of the code generation (it just generate comments though 😅)

needs:
- setup
runs-on: ${{ fromJSON(needs.setup.outputs.compute-large) }}
steps:
- uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
# NOTE: This step is specifically needed for ENT. It allows us to access the required private HashiCorp repos.
- name: Setup Git
if: ${{ endsWith(github.repository, '-enterprise') }}
run: git config --global url."https://${{ secrets.ELEVATED_GITHUB_TOKEN }}:@github.com".insteadOf "https://github.com"
- uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1
with:
go-version-file: 'go.mod'
- run: make --always-make codegen
- run: |
if ! git diff --exit-code; then
echo "Generated code was not updated correctly"
exit 1
fi
- name: Notify Slack
if: ${{ failure() }}
run: .github/scripts/notify_slack.sh

lint-enums:
needs:
Expand Down Expand Up @@ -486,8 +485,7 @@ jobs:
needs:
- conditional-skip
- setup
# Reenable later
#- check-generated-deep-copy
- check-codegen
- check-generated-protobuf
- check-go-mod
- lint-consul-retry
Expand Down
6 changes: 4 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ MOG_VERSION='v0.4.0'
PROTOC_GO_INJECT_TAG_VERSION='v1.3.0'
PROTOC_GEN_GO_BINARY_VERSION="v0.1.0"
DEEP_COPY_VERSION='bc3f5aa5735d8a54961580a3a24422c308c831c2'
COPYWRITE_TOOL_VERSION='v0.16.4'

MOCKED_PB_DIRS= pbdns

Expand Down Expand Up @@ -427,12 +428,13 @@ lint-tools: ## Install tools for linting
codegen-tools: ## Install tools for codegen
@$(SHELL) $(CURDIR)/build-support/scripts/devtools.sh -codegen

.PHONY: deep-copy
deep-copy: codegen-tools ## Deep copy
.PHONY: codegen
codegen: codegen-tools ## Deep copy
@$(SHELL) $(CURDIR)/agent/structs/deep-copy.sh
@$(SHELL) $(CURDIR)/agent/proxycfg/deep-copy.sh
@$(SHELL) $(CURDIR)/agent/consul/state/deep-copy.sh
@$(SHELL) $(CURDIR)/agent/config/deep-copy.sh
copywrite headers
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to limit this to just *.deepcopy.go so that other files aren't caught by the check? Maybe there is cause to refactor this to check to all files unless we're already doing that somewhere else.

Copy link
Collaborator Author

@dhiaayachi dhiaayachi Sep 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We are not doing this to all the files right now. My thinking is that we should, as I already found out few files which don't have the copywrite header.
That said, maybe it would be better to make it independent from deep-copy 🤔


print-% : ; @echo $($*) ## utility to echo a makefile variable (i.e. 'make print-GOPATH')

Expand Down
3 changes: 3 additions & 0 deletions agent/config/config.deepcopy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

// generated by deep-copy -pointer-receiver -o ./config.deepcopy.go -type RuntimeConfig ./; DO NOT EDIT.

package config
Expand Down
3 changes: 3 additions & 0 deletions agent/proxycfg-sources/catalog/config_source_oss.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

//go:build !consulent
// +build !consulent

Expand Down
3 changes: 3 additions & 0 deletions agent/proxycfg/config_snapshot_glue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package proxycfg

import (
Expand Down
3 changes: 3 additions & 0 deletions agent/proxycfg/config_snapshot_glue_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package proxycfg

import (
Expand Down
29 changes: 23 additions & 6 deletions build-support/scripts/devtools.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,21 +149,38 @@ function lint_install {
}

function codegen_install {
local deep_copy_version
deep_copy_version="$(make --no-print-directory print-DEEP_COPY_VERSION)"
deepcopy_install
copywrite_install
}

function deepcopy_install {
local deep_copy_version
deep_copy_version="$(make --no-print-directory print-DEEP_COPY_VERSION)"

install_versioned_tool \
'deep-copy' \
'github.com/globusdigital/deep-copy' \
"${deep_copy_version}" \
'github.com/globusdigital/deep-copy'
}

function copywrite_install {
local copywrite_version
copywrite_version="$(make --no-print-directory print-COPYWRITE_TOOL_VERSION)"

install_versioned_tool \
'deep-copy' \
'github.com/globusdigital/deep-copy' \
"${deep_copy_version}" \
'github.com/globusdigital/deep-copy'
'copywrite' \
'github.com/hashicorp/copywrite' \
"${copywrite_version}" \
'github.com/hashicorp/copywrite'
}

function tools_install {

lint_install
proto_tools_install
codegen_install
copywrite_install

return 0
}
Expand Down
3 changes: 3 additions & 0 deletions command/resource/testdata/demo.hcl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# Copyright (c) HashiCorp, Inc.
# SPDX-License-Identifier: BUSL-1.1

ID {
Type = gvk("demo.v2.Artist")
Name = "korn"
Expand Down
3 changes: 3 additions & 0 deletions command/resource/testdata/invalid.hcl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# Copyright (c) HashiCorp, Inc.
# SPDX-License-Identifier: BUSL-1.1

ID {
Type = gvk("demo.v2.Artist")
Name = "korn"
Expand Down
3 changes: 3 additions & 0 deletions command/resource/testdata/invalid_type.hcl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# Copyright (c) HashiCorp, Inc.
# SPDX-License-Identifier: BUSL-1.1

D {
Type = gvk("demo.v2.Artist")
Tenancy {
Expand Down
3 changes: 3 additions & 0 deletions command/resource/testdata/nested_data.hcl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# Copyright (c) HashiCorp, Inc.
# SPDX-License-Identifier: BUSL-1.1

ID {
Type = gvk("mesh.v1alpha1.Upstreams")
Name = "api"
Expand Down
3 changes: 3 additions & 0 deletions internal/mesh/internal/cache/sidecarproxycache/destinations_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package sidecarproxycache

import (
Expand Down
3 changes: 3 additions & 0 deletions internal/mesh/internal/cache/sidecarproxycache/destinations_cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package sidecarproxycache

import (
Expand Down
3 changes: 3 additions & 0 deletions internal/mesh/internal/controllers/xds/proxy_tracker_watch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package xds

import (
Expand Down
3 changes: 3 additions & 0 deletions internal/mesh/proxy-snapshot/proxy_snapshot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package proxysnapshot

import "github.com/hashicorp/consul/acl"
Expand Down
3 changes: 3 additions & 0 deletions internal/mesh/proxy-tracker/proxy_state_exports.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package proxytracker

import (
Expand Down
3 changes: 3 additions & 0 deletions logging/logfile_bsd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

//go:build darwin || freebsd || netbsd || openbsd
// +build darwin freebsd netbsd openbsd

Expand Down
3 changes: 3 additions & 0 deletions logging/logfile_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

//go:build dragonfly || linux
// +build dragonfly linux

Expand Down
3 changes: 3 additions & 0 deletions logging/logfile_solaris.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

//go:build solaris
// +build solaris

Expand Down
3 changes: 3 additions & 0 deletions logging/logfile_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package logging

import (
Expand Down
6 changes: 3 additions & 3 deletions test-integ/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ require (
github.com/googleapis/gax-go/v2 v2.11.0 // indirect
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.11.3 // indirect
github.com/hashicorp/consul v0.0.0-00010101000000-000000000000 // indirect
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is not really related but I decided to add it, as it should not matter and my IDE annoyingly update this every time I make a change and I'm pretty sure I'm not the only one, I've seen it in others setup.

github.com/hashicorp/consul v1.16.1 // indirect
github.com/hashicorp/consul-awsauth v0.0.0-20220713182709-05ac1c5c2706 // indirect
github.com/hashicorp/consul-net-rpc v0.0.0-20221205195236-156cfab66a69 // indirect
github.com/hashicorp/consul/envoyextensions v0.4.1 // indirect
Expand Down Expand Up @@ -204,7 +204,7 @@ require (
go.opentelemetry.io/proto/otlp v0.19.0 // indirect
go.uber.org/atomic v1.9.0 // indirect
golang.org/x/crypto v0.12.0 // indirect
golang.org/x/exp v0.0.0-20230801115018-d63ba01acd4b // indirect
golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63 // indirect
golang.org/x/mod v0.12.0 // indirect
golang.org/x/net v0.14.0 // indirect
golang.org/x/oauth2 v0.8.0 // indirect
Expand All @@ -213,7 +213,7 @@ require (
golang.org/x/term v0.11.0 // indirect
golang.org/x/text v0.12.0 // indirect
golang.org/x/time v0.3.0 // indirect
golang.org/x/tools v0.11.1 // indirect
golang.org/x/tools v0.12.1-0.20230815132531-74c255bcf846 // indirect
google.golang.org/api v0.126.0 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20230726155614-23370e0ffb3e // indirect
Expand Down
8 changes: 4 additions & 4 deletions test-integ/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,8 +853,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
golang.org/x/exp v0.0.0-20230801115018-d63ba01acd4b h1:r+vk0EmXNmekl0S0BascoeeoHk/L7wmaW2QF90K+kYI=
golang.org/x/exp v0.0.0-20230801115018-d63ba01acd4b/go.mod h1:FXUEEKJgO7OQYeo8N01OfiKP8RXMtf6e8aTskBGqWdc=
golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63 h1:m64FZMko/V45gv0bNmrNYoDEq8U5YUhetc9cBWKS1TQ=
golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63/go.mod h1:0v4NqG35kSWCMzLaMeX+IQrlSnVE/bqGSyC2cz/9Le8=
golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
Expand Down Expand Up @@ -1126,8 +1126,8 @@ golang.org/x/tools v0.1.4/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/tools v0.1.6-0.20210726203631-07bc1bf47fb2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
golang.org/x/tools v0.11.1 h1:ojD5zOW8+7dOGzdnNgersm8aPfcDjhMp12UfG93NIMc=
golang.org/x/tools v0.11.1/go.mod h1:anzJrxPjNtfgiYQYirP2CPGzGLxrH2u2QBhn6Bf3qY8=
golang.org/x/tools v0.12.1-0.20230815132531-74c255bcf846 h1:Vve/L0v7CXXuxUmaMGIEK/dEeq7uiqb5qBgQrZzIE7E=
golang.org/x/tools v0.12.1-0.20230815132531-74c255bcf846/go.mod h1:Sc0INKfu04TlqNoRA1hgpFZbhYXHPr4V5DzpSBTPqQM=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
Expand Down