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

Remove the need for custom Make targets #1292

Merged
merged 10 commits into from
Jan 16, 2025
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
24 changes: 19 additions & 5 deletions provider-ci/internal/pkg/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,16 +274,30 @@ type Config struct {
//
// This function is responsible for creating a provider binary for the desired platform.
//
// Use "$(1)" to refer to the desired output location.
// "$(1)" refers to the desired OS, in the same format as GOOS in the Go toolchain
// "$(2)" refers to the desired architecture, in the same format as GOARCH in the Go toolchain
// "$(3)" refers to the desired destination path for the binary
//
// If a cross-compiling build is requested, the environment will have GOOS and GOARCH set.
// An example value for the command is:
//
// The default value uses go build:
//
// cd provider && go build -o "$(1)" ...
// cd provider && GOOS=$(1) GOARCH=$(2) go build -o "$(3)" ...
//
// Customizing this value allows providers implemented in Node or other languages.
BuildProviderCmd string `yaml:"buildProviderCmd"`

// Customizes a hook to run right before BuildProviderCmd.
BuildProviderPre string `yaml:"buildProviderPre"`

// Customizes the Make function test_provider_cmd.
//
// This function is called without arguments to run unit tests for the provider binary.
TestProviderCmd string `yaml:"testProviderCmd"`

// Customizes the Make function renovate_cmd.
//
// This function is called by the make renovate target after the Renovate bot has finished updating
// project dependencies in a PR. It is responsible for rebuilding any generated files as needed.
RenovateCmd string `yaml:"renovateCmd"`
}

// LoadLocalConfig loads the provider configuration at the given path with
Expand Down
66 changes: 43 additions & 23 deletions provider-ci/internal/pkg/templates/bridged-provider/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -255,32 +255,45 @@ lint_provider.fix:
cd provider && golangci-lint run --path-prefix provider -c ../.golangci.yml --fix
.PHONY: lint_provider lint_provider.fix

# `make provider_no_deps` builds the provider binary directly, without ensuring that
# `cmd/pulumi-resource-#{{ .Config.Provider }}#/schema.json` is valid and up to date.
# To create a release ready binary, you should use `make provider`.
#{{- if .Config.BuildProviderCmd }}#
build_provider_cmd = #{{ .Config.BuildProviderCmd }}#
build_provider_cmd = #{{ if .Config.BuildProviderPre -}}#
#{{ .Config.BuildProviderPre }}#;
#{{- end -}}#
#{{ .Config.BuildProviderCmd }}#
#{{- else }}#
build_provider_cmd = cd provider && CGO_ENABLED=0 go build $(PULUMI_PROVIDER_BUILD_PARALLELISM) -o "$(1)" -ldflags "$(LDFLAGS)" $(PROJECT)/$(PROVIDER_PATH)/cmd/$(PROVIDER)
build_provider_cmd = #{{ if .Config.BuildProviderPre -}}#
#{{ .Config.BuildProviderPre }}#;
#{{- end -}}#
cd provider && GOOS=$(1) GOARCH=$(2) CGO_ENABLED=0 go build $(PULUMI_PROVIDER_BUILD_PARALLELISM) -o "$(3)" -ldflags "$(LDFLAGS)" $(PROJECT)/$(PROVIDER_PATH)/cmd/$(PROVIDER)
#{{- end }}#

provider: bin/$(PROVIDER)

# `make provider_no_deps` builds the provider binary directly, without ensuring that
# `cmd/pulumi-resource-#{{ .Config.Provider }}#/schema.json` is valid and up to date.
# To create a release ready binary, you should use `make provider`.
provider_no_deps:
$(call build_provider_cmd,$(WORKING_DIR)/bin/$(PROVIDER))
$(call build_provider_cmd,$(shell go env GOOS),$(shell go env GOARCH),$(WORKING_DIR)/bin/$(PROVIDER))
bin/$(PROVIDER): .make/schema
$(call build_provider_cmd,$(WORKING_DIR)/bin/$(PROVIDER))
$(call build_provider_cmd,$(shell go env GOOS),$(shell go env GOARCH),$(WORKING_DIR)/bin/$(PROVIDER))
.PHONY: provider provider_no_deps

test: export PATH := $(WORKING_DIR)/bin:$(PATH)
test:
cd examples && go test -v -tags=all -parallel $(TESTPARALLELISM) -timeout 2h
.PHONY: test

#{{- if .Config.TestProviderCmd }}#
test_provider_cmd = #{{ .Config.TestProviderCmd }}#
#{{- else }}#
test_provider_cmd = cd provider && go test -v -short \
-coverprofile="coverage.txt" \
-coverpkg="./...,github.com/hashicorp/terraform-provider-..." \
-parallel $(TESTPARALLELISM) \
./...
#{{- end }}#
test_provider:
cd provider && go test -v -short \
-coverprofile="coverage.txt" \
-coverpkg="./...,github.com/hashicorp/terraform-provider-..." \
-parallel $(TESTPARALLELISM) \
./...
$(call test_provider_cmd)
.PHONY: test_provider

tfgen: schema
Expand Down Expand Up @@ -370,18 +383,18 @@ SKIP_SIGNING ?=

# These targets assume that the schema-embed.json exists - it's generated by tfgen.
# We disable CGO to ensure that the binary is statically linked.
bin/linux-amd64/$(PROVIDER): export GOOS := linux
bin/linux-amd64/$(PROVIDER): export GOARCH := amd64
bin/linux-arm64/$(PROVIDER): export GOOS := linux
bin/linux-arm64/$(PROVIDER): export GOARCH := arm64
bin/darwin-amd64/$(PROVIDER): export GOOS := darwin
bin/darwin-amd64/$(PROVIDER): export GOARCH := amd64
bin/darwin-arm64/$(PROVIDER): export GOOS := darwin
bin/darwin-arm64/$(PROVIDER): export GOARCH := arm64
bin/windows-amd64/$(PROVIDER).exe: export GOOS := windows
bin/windows-amd64/$(PROVIDER).exe: export GOARCH := amd64
bin/linux-amd64/$(PROVIDER): GOOS := linux
bin/linux-amd64/$(PROVIDER): GOARCH := amd64
bin/linux-arm64/$(PROVIDER): GOOS := linux
Copy link
Member Author

Choose a reason for hiding this comment

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

Found that export transitively affects all sub-targets, which is unfortunate and not needed. Passing it more precisely is more predictable.

bin/linux-arm64/$(PROVIDER): GOARCH := arm64
bin/darwin-amd64/$(PROVIDER): GOOS := darwin
bin/darwin-amd64/$(PROVIDER): GOARCH := amd64
bin/darwin-arm64/$(PROVIDER): GOOS := darwin
bin/darwin-arm64/$(PROVIDER): GOARCH := arm64
bin/windows-amd64/$(PROVIDER).exe: GOOS := windows
bin/windows-amd64/$(PROVIDER).exe: GOARCH := amd64
bin/%/$(PROVIDER) bin/%/$(PROVIDER).exe: bin/jsign-6.0.jar
$(call build_provider_cmd,$(WORKING_DIR)/$@)
$(call build_provider_cmd,$(GOOS),$(GOARCH),$(WORKING_DIR)/$@)

@# Only sign windows binary if fully configured.
@# Test variables set by joining with | between and looking for || showing at least one variable is empty.
Expand Down Expand Up @@ -440,5 +453,12 @@ provider_dist-windows-amd64: bin/$(PROVIDER)-v$(VERSION_GENERIC)-windows-amd64.t
provider_dist: provider_dist-linux-amd64 provider_dist-linux-arm64 provider_dist-darwin-amd64 provider_dist-darwin-arm64 provider_dist-windows-amd64
.PHONY: provider_dist-linux-amd64 provider_dist-linux-arm64 provider_dist-darwin-amd64 provider_dist-darwin-arm64 provider_dist-windows-amd64 provider_dist

#{{- if .Config.RenovateCmd }}#
renovate_cmd = #{{ .Config.RenovateCmd }}#
renovate:
$(call renovate_cmd)
.PHONY: renovate
#{{- end }}#
Comment on lines +456 to +461
Copy link
Member

Choose a reason for hiding this comment

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

As discussed, I think this is a backwards way of configuring renovate - we should just be building an existing makefile target via the per-repo renovate config instead.

The renovate target is fine as a temporary workaround until we figure out why renovate won't let us call make generate.

One suggestion here for simplifying would be to not make it an arbirary command but make it .Config.RenovateTarget instead so it's not arbitary code, but just an alias to an existing target.

This would also simplify the readability of the code as it's just creating an alias to an existing target:

Suggested change
#{{- if .Config.RenovateCmd }}#
renovate_cmd = #{{ .Config.RenovateCmd }}#
renovate:
$(call renovate_cmd)
.PHONY: renovate
#{{- end }}#
#{{- if .Config.RenovateTarget }}#
renovate: #{{ .Config.RenovateTarget }}#
#{{- end }}#

Having the configuration as an existing target will also make it trivial in the future to remove this target and instead generate the renovate config with the call to make #{{ .Config.RenovateTarget }}# directly.

Copy link
Member Author

Choose a reason for hiding this comment

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

I like make renovate target TBH. This makes the interface between sub-systems more about the intent and less about the implementation which feels good.

Copy link
Member Author

Choose a reason for hiding this comment

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

The suggested change does make https://github.com/pulumi/pulumi-awsx/pull/1485/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R381 more difficult - we're not just generating SDKs, renovate might need to do something else specific to the repo. I think I will leave the Cmd hook. The Target hook seems to invite backdoors such as:

Config.RenovateTarget = "generate_sdks\n\techo STUFF\n\npost_renovate: renovate"


# Permit providers to extend the Makefile with provider-specific Make includes.
include $(wildcard .mk/*.mk)
43 changes: 22 additions & 21 deletions provider-ci/test-providers/acme/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -214,29 +214,30 @@ lint_provider: provider
lint_provider.fix:
cd provider && golangci-lint run --path-prefix provider -c ../.golangci.yml --fix
.PHONY: lint_provider lint_provider.fix
build_provider_cmd = cd provider && GOOS=$(1) GOARCH=$(2) CGO_ENABLED=0 go build $(PULUMI_PROVIDER_BUILD_PARALLELISM) -o "$(3)" -ldflags "$(LDFLAGS)" $(PROJECT)/$(PROVIDER_PATH)/cmd/$(PROVIDER)

provider: bin/$(PROVIDER)

# `make provider_no_deps` builds the provider binary directly, without ensuring that
# `cmd/pulumi-resource-acme/schema.json` is valid and up to date.
# To create a release ready binary, you should use `make provider`.
build_provider_cmd = cd provider && CGO_ENABLED=0 go build $(PULUMI_PROVIDER_BUILD_PARALLELISM) -o "$(1)" -ldflags "$(LDFLAGS)" $(PROJECT)/$(PROVIDER_PATH)/cmd/$(PROVIDER)
provider: bin/$(PROVIDER)
provider_no_deps:
$(call build_provider_cmd,$(WORKING_DIR)/bin/$(PROVIDER))
$(call build_provider_cmd,$(shell go env GOOS),$(shell go env GOARCH),$(WORKING_DIR)/bin/$(PROVIDER))
bin/$(PROVIDER): .make/schema
$(call build_provider_cmd,$(WORKING_DIR)/bin/$(PROVIDER))
$(call build_provider_cmd,$(shell go env GOOS),$(shell go env GOARCH),$(WORKING_DIR)/bin/$(PROVIDER))
.PHONY: provider provider_no_deps

test: export PATH := $(WORKING_DIR)/bin:$(PATH)
test:
cd examples && go test -v -tags=all -parallel $(TESTPARALLELISM) -timeout 2h
.PHONY: test

test_provider_cmd = cd provider && go test -v -short \
-coverprofile="coverage.txt" \
-coverpkg="./...,github.com/hashicorp/terraform-provider-..." \
-parallel $(TESTPARALLELISM) \
./...
test_provider:
cd provider && go test -v -short \
-coverprofile="coverage.txt" \
-coverpkg="./...,github.com/hashicorp/terraform-provider-..." \
-parallel $(TESTPARALLELISM) \
./...
$(call test_provider_cmd)
.PHONY: test_provider

tfgen: schema
Expand Down Expand Up @@ -315,18 +316,18 @@ SKIP_SIGNING ?=

# These targets assume that the schema-embed.json exists - it's generated by tfgen.
# We disable CGO to ensure that the binary is statically linked.
bin/linux-amd64/$(PROVIDER): export GOOS := linux
bin/linux-amd64/$(PROVIDER): export GOARCH := amd64
bin/linux-arm64/$(PROVIDER): export GOOS := linux
bin/linux-arm64/$(PROVIDER): export GOARCH := arm64
bin/darwin-amd64/$(PROVIDER): export GOOS := darwin
bin/darwin-amd64/$(PROVIDER): export GOARCH := amd64
bin/darwin-arm64/$(PROVIDER): export GOOS := darwin
bin/darwin-arm64/$(PROVIDER): export GOARCH := arm64
bin/windows-amd64/$(PROVIDER).exe: export GOOS := windows
bin/windows-amd64/$(PROVIDER).exe: export GOARCH := amd64
bin/linux-amd64/$(PROVIDER): GOOS := linux
bin/linux-amd64/$(PROVIDER): GOARCH := amd64
bin/linux-arm64/$(PROVIDER): GOOS := linux
bin/linux-arm64/$(PROVIDER): GOARCH := arm64
bin/darwin-amd64/$(PROVIDER): GOOS := darwin
bin/darwin-amd64/$(PROVIDER): GOARCH := amd64
bin/darwin-arm64/$(PROVIDER): GOOS := darwin
bin/darwin-arm64/$(PROVIDER): GOARCH := arm64
bin/windows-amd64/$(PROVIDER).exe: GOOS := windows
bin/windows-amd64/$(PROVIDER).exe: GOARCH := amd64
bin/%/$(PROVIDER) bin/%/$(PROVIDER).exe: bin/jsign-6.0.jar
$(call build_provider_cmd,$(WORKING_DIR)/$@)
$(call build_provider_cmd,$(GOOS),$(GOARCH),$(WORKING_DIR)/$@)

@# Only sign windows binary if fully configured.
@# Test variables set by joining with | between and looking for || showing at least one variable is empty.
Expand Down
43 changes: 22 additions & 21 deletions provider-ci/test-providers/aws/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -224,29 +224,30 @@ lint_provider: provider
lint_provider.fix:
cd provider && golangci-lint run --path-prefix provider -c ../.golangci.yml --fix
.PHONY: lint_provider lint_provider.fix
build_provider_cmd = cd provider && GOOS=$(1) GOARCH=$(2) CGO_ENABLED=0 go build $(PULUMI_PROVIDER_BUILD_PARALLELISM) -o "$(3)" -ldflags "$(LDFLAGS)" $(PROJECT)/$(PROVIDER_PATH)/cmd/$(PROVIDER)

provider: bin/$(PROVIDER)

# `make provider_no_deps` builds the provider binary directly, without ensuring that
# `cmd/pulumi-resource-aws/schema.json` is valid and up to date.
# To create a release ready binary, you should use `make provider`.
build_provider_cmd = cd provider && CGO_ENABLED=0 go build $(PULUMI_PROVIDER_BUILD_PARALLELISM) -o "$(1)" -ldflags "$(LDFLAGS)" $(PROJECT)/$(PROVIDER_PATH)/cmd/$(PROVIDER)
provider: bin/$(PROVIDER)
provider_no_deps:
$(call build_provider_cmd,$(WORKING_DIR)/bin/$(PROVIDER))
$(call build_provider_cmd,$(shell go env GOOS),$(shell go env GOARCH),$(WORKING_DIR)/bin/$(PROVIDER))
bin/$(PROVIDER): .make/schema
$(call build_provider_cmd,$(WORKING_DIR)/bin/$(PROVIDER))
$(call build_provider_cmd,$(shell go env GOOS),$(shell go env GOARCH),$(WORKING_DIR)/bin/$(PROVIDER))
.PHONY: provider provider_no_deps

test: export PATH := $(WORKING_DIR)/bin:$(PATH)
test:
cd examples && go test -v -tags=all -parallel $(TESTPARALLELISM) -timeout 2h
.PHONY: test

test_provider_cmd = cd provider && go test -v -short \
-coverprofile="coverage.txt" \
-coverpkg="./...,github.com/hashicorp/terraform-provider-..." \
-parallel $(TESTPARALLELISM) \
./...
test_provider:
cd provider && go test -v -short \
-coverprofile="coverage.txt" \
-coverpkg="./...,github.com/hashicorp/terraform-provider-..." \
-parallel $(TESTPARALLELISM) \
./...
$(call test_provider_cmd)
.PHONY: test_provider

tfgen: schema
Expand Down Expand Up @@ -329,18 +330,18 @@ SKIP_SIGNING ?=

# These targets assume that the schema-embed.json exists - it's generated by tfgen.
# We disable CGO to ensure that the binary is statically linked.
bin/linux-amd64/$(PROVIDER): export GOOS := linux
bin/linux-amd64/$(PROVIDER): export GOARCH := amd64
bin/linux-arm64/$(PROVIDER): export GOOS := linux
bin/linux-arm64/$(PROVIDER): export GOARCH := arm64
bin/darwin-amd64/$(PROVIDER): export GOOS := darwin
bin/darwin-amd64/$(PROVIDER): export GOARCH := amd64
bin/darwin-arm64/$(PROVIDER): export GOOS := darwin
bin/darwin-arm64/$(PROVIDER): export GOARCH := arm64
bin/windows-amd64/$(PROVIDER).exe: export GOOS := windows
bin/windows-amd64/$(PROVIDER).exe: export GOARCH := amd64
bin/linux-amd64/$(PROVIDER): GOOS := linux
bin/linux-amd64/$(PROVIDER): GOARCH := amd64
bin/linux-arm64/$(PROVIDER): GOOS := linux
bin/linux-arm64/$(PROVIDER): GOARCH := arm64
bin/darwin-amd64/$(PROVIDER): GOOS := darwin
bin/darwin-amd64/$(PROVIDER): GOARCH := amd64
bin/darwin-arm64/$(PROVIDER): GOOS := darwin
bin/darwin-arm64/$(PROVIDER): GOARCH := arm64
bin/windows-amd64/$(PROVIDER).exe: GOOS := windows
bin/windows-amd64/$(PROVIDER).exe: GOARCH := amd64
bin/%/$(PROVIDER) bin/%/$(PROVIDER).exe: bin/jsign-6.0.jar
$(call build_provider_cmd,$(WORKING_DIR)/$@)
$(call build_provider_cmd,$(GOOS),$(GOARCH),$(WORKING_DIR)/$@)

@# Only sign windows binary if fully configured.
@# Test variables set by joining with | between and looking for || showing at least one variable is empty.
Expand Down
43 changes: 22 additions & 21 deletions provider-ci/test-providers/cloudflare/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -224,29 +224,30 @@ lint_provider: provider
lint_provider.fix:
cd provider && golangci-lint run --path-prefix provider -c ../.golangci.yml --fix
.PHONY: lint_provider lint_provider.fix
build_provider_cmd = cd provider && GOOS=$(1) GOARCH=$(2) CGO_ENABLED=0 go build $(PULUMI_PROVIDER_BUILD_PARALLELISM) -o "$(3)" -ldflags "$(LDFLAGS)" $(PROJECT)/$(PROVIDER_PATH)/cmd/$(PROVIDER)

provider: bin/$(PROVIDER)

# `make provider_no_deps` builds the provider binary directly, without ensuring that
# `cmd/pulumi-resource-cloudflare/schema.json` is valid and up to date.
# To create a release ready binary, you should use `make provider`.
build_provider_cmd = cd provider && CGO_ENABLED=0 go build $(PULUMI_PROVIDER_BUILD_PARALLELISM) -o "$(1)" -ldflags "$(LDFLAGS)" $(PROJECT)/$(PROVIDER_PATH)/cmd/$(PROVIDER)
provider: bin/$(PROVIDER)
provider_no_deps:
$(call build_provider_cmd,$(WORKING_DIR)/bin/$(PROVIDER))
$(call build_provider_cmd,$(shell go env GOOS),$(shell go env GOARCH),$(WORKING_DIR)/bin/$(PROVIDER))
bin/$(PROVIDER): .make/schema
$(call build_provider_cmd,$(WORKING_DIR)/bin/$(PROVIDER))
$(call build_provider_cmd,$(shell go env GOOS),$(shell go env GOARCH),$(WORKING_DIR)/bin/$(PROVIDER))
.PHONY: provider provider_no_deps

test: export PATH := $(WORKING_DIR)/bin:$(PATH)
test:
cd examples && go test -v -tags=all -parallel $(TESTPARALLELISM) -timeout 2h
.PHONY: test

test_provider_cmd = cd provider && go test -v -short \
-coverprofile="coverage.txt" \
-coverpkg="./...,github.com/hashicorp/terraform-provider-..." \
-parallel $(TESTPARALLELISM) \
./...
test_provider:
cd provider && go test -v -short \
-coverprofile="coverage.txt" \
-coverpkg="./...,github.com/hashicorp/terraform-provider-..." \
-parallel $(TESTPARALLELISM) \
./...
$(call test_provider_cmd)
.PHONY: test_provider

tfgen: schema
Expand Down Expand Up @@ -325,18 +326,18 @@ SKIP_SIGNING ?=

# These targets assume that the schema-embed.json exists - it's generated by tfgen.
# We disable CGO to ensure that the binary is statically linked.
bin/linux-amd64/$(PROVIDER): export GOOS := linux
bin/linux-amd64/$(PROVIDER): export GOARCH := amd64
bin/linux-arm64/$(PROVIDER): export GOOS := linux
bin/linux-arm64/$(PROVIDER): export GOARCH := arm64
bin/darwin-amd64/$(PROVIDER): export GOOS := darwin
bin/darwin-amd64/$(PROVIDER): export GOARCH := amd64
bin/darwin-arm64/$(PROVIDER): export GOOS := darwin
bin/darwin-arm64/$(PROVIDER): export GOARCH := arm64
bin/windows-amd64/$(PROVIDER).exe: export GOOS := windows
bin/windows-amd64/$(PROVIDER).exe: export GOARCH := amd64
bin/linux-amd64/$(PROVIDER): GOOS := linux
bin/linux-amd64/$(PROVIDER): GOARCH := amd64
bin/linux-arm64/$(PROVIDER): GOOS := linux
bin/linux-arm64/$(PROVIDER): GOARCH := arm64
bin/darwin-amd64/$(PROVIDER): GOOS := darwin
bin/darwin-amd64/$(PROVIDER): GOARCH := amd64
bin/darwin-arm64/$(PROVIDER): GOOS := darwin
bin/darwin-arm64/$(PROVIDER): GOARCH := arm64
bin/windows-amd64/$(PROVIDER).exe: GOOS := windows
bin/windows-amd64/$(PROVIDER).exe: GOARCH := amd64
bin/%/$(PROVIDER) bin/%/$(PROVIDER).exe: bin/jsign-6.0.jar
$(call build_provider_cmd,$(WORKING_DIR)/$@)
$(call build_provider_cmd,$(GOOS),$(GOARCH),$(WORKING_DIR)/$@)

@# Only sign windows binary if fully configured.
@# Test variables set by joining with | between and looking for || showing at least one variable is empty.
Expand Down
Loading
Loading