feat: Add global build pre- and post-hooks

[ar3s3ru]

Description
This PR adds the possibility of running pre- and post-hooks for the Build phase of a Pipeline.

The implementation is done by leveraging the pre-existing build.PipelineBuilder interface, using a wrapper that extends PreBuild and PostBuild methods, to avoid significant changes to the pre-existing code.

User facing changes

This PR adds the new functionality by introducing a new build.hooks field in the configuration.

Due to the new configuration field, I've created a new version, v4beta7, as specified in the contribution guidelines.

This is a skaffold build run from a slightly modified version of examples/getting-started/skaffold.yaml:

# skaffold.yaml
apiVersion: skaffold/v4beta7
kind: Config
build:
  hooks:
    before:
      - command: [echo, "hello from pre-hook!"]
    after:
      - command: [echo, "hello from post-hook!"]
  artifacts:
    - image: skaffold-example
  local:
    push: false
manifests:
  rawYaml:
  - k8s-pod.yaml

Here is the output, which shows how the new build.hooks is behaving:

> ./out/skaffold build
Generating tags...
 - skaffold-example -> skaffold-example:740d30e8f-dirty
Checking cache...
 - skaffold-example: Not found. Building
Starting build...
Starting pre-build hooks...
hello from pre-hook!
Completed pre-build hooks
Building [skaffold-example]...
Sending build context to Docker daemon  4.096kB
time="2023-08-27T18:06:54+02:00" level=warning msg="missing \"SKAFFOLD_GO_GCFLAGS\" build argument. Try adding \"--build-arg SKAFFOLD_GO_GCFLAGS=<VALUE>\" to the command line"
[1/2] STEP 1/6: FROM golang:1.18 AS builder
[1/2] STEP 2/6: WORKDIR /code
--> Using cache bffc72dc309d959a859d59a0aae3975abb0369b1420fc66551dbee53abe9670c
--> bffc72dc309d
[1/2] STEP 3/6: COPY main.go .
--> Using cache e9ae2def9ed085da743cddbb2dcc092a7302dc8dff53955dbd9508f166290bbe
--> e9ae2def9ed0
[1/2] STEP 4/6: COPY go.mod .
--> Using cache a22d7afc1bd669d99500bbbadd550343c390f8460a7b0767ff2c48b47de4961d
--> a22d7afc1bd6
[1/2] STEP 5/6: ARG SKAFFOLD_GO_GCFLAGS
--> Using cache 3c1e21609fff4dc761893cac7e2087705c05dc57ed84acbe4a00d13d56164f72
--> 3c1e21609fff
[1/2] STEP 6/6: RUN go build -gcflags="${SKAFFOLD_GO_GCFLAGS}" -trimpath -o /app main.go
--> Using cache c082cefdd6ef749c854636c91f03c3098f98c13db26e81471d8461ad73a86cea
--> c082cefdd6ef
[2/2] STEP 1/4: FROM alpine:3
[2/2] STEP 2/4: ENV GOTRACEBACK=single
--> Using cache 3df39876b4561aff7cf01adab4e0535b52b7b686bea9954c77c1d4923d9f5388
--> 3df39876b456
[2/2] STEP 3/4: CMD ["./app"]
--> Using cache 4fd318e64452faa0c2461f254ee15b2bd35b9ded3439f21db8bdd9296f73c189
--> 4fd318e64452
[2/2] STEP 4/4: COPY --from=builder /app .
--> Using cache 80a5e519d652de8a28d5466706eaf6cc77be7a64ffdb358189245370f8605ca1
[2/2] COMMIT docker.io/library/skaffold-example:740d30e8f-dirty
--> 80a5e519d652
Successfully tagged docker.io/library/skaffold-example:740d30e8f-dirty
80a5e519d652de8a28d5466706eaf6cc77be7a64ffdb358189245370f8605ca1
Successfully built 80a5e519d652
Successfully tagged skaffold-example:740d30e8f-dirty
Build [skaffold-example] succeeded
Starting post-build hooks...
hello from post-hook!
Completed post-build hooks

[ar3s3ru]

Note for reviewers: I am a bit lost on the testing strategy.
Which kind of tests do you suggest for this new feature? Some pointer will be greatly appreciated 🙌🏻

What's more, creating a new version introduced a ton of changes -- is that ok?

[ar3s3ru]

Rebased to latest main since #9054 has been merged and brought in a new v4beta7 version.

[ericzzzzzzz]

Hi @ar3s3ru
thank you for the contribution, here is an example

skaffold/pkg/skaffold/hooks/render_test.go

Lines 1 to 75 in 88e1c17

	/*
	Copyright 2021 The Skaffold Authors
	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.
	*/
	package hooks
	import (
	"bytes"
	"context"
	"testing"
	kubernetesclient "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/kubernetes/client"
	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest"
	"github.com/GoogleContainerTools/skaffold/v2/testutil"
	)
	func TestRenderHooks(t *testing.T) {
	testutil.Run(t, "TestRenderHooks", func(t *testutil.T) {
	hooks := latest.RenderHooks{
	PreHooks: []latest.RenderHookItem{
	{
	HostHook: &latest.HostHook{
	OS: []string{"linux", "darwin"},
	Command: []string{"sh", "-c", "echo pre-hook running with SKAFFOLD_KUBE_CONTEXT=$SKAFFOLD_KUBE_CONTEXT,SKAFFOLD_NAMESPACES=$SKAFFOLD_NAMESPACES"},
	},
	},
	{
	HostHook: &latest.HostHook{
	OS: []string{"windows"},
	Command: []string{"cmd.exe", "/C", "echo pre-hook running with SKAFFOLD_KUBE_CONTEXT=%SKAFFOLD_KUBE_CONTEXT%,SKAFFOLD_NAMESPACES=%SKAFFOLD_NAMESPACES%"},
	},
	},
	},
	PostHooks: []latest.RenderHookItem{
	{
	HostHook: &latest.HostHook{
	OS: []string{"linux", "darwin"},
	Command: []string{"sh", "-c", "echo post-hook running with SKAFFOLD_KUBE_CONTEXT=$SKAFFOLD_KUBE_CONTEXT,SKAFFOLD_NAMESPACES=$SKAFFOLD_NAMESPACES"},
	},
	},
	{
	HostHook: &latest.HostHook{
	OS: []string{"windows"},
	Command: []string{"cmd.exe", "/C", "echo post-hook running with SKAFFOLD_KUBE_CONTEXT=%SKAFFOLD_KUBE_CONTEXT%,SKAFFOLD_NAMESPACES=%SKAFFOLD_NAMESPACES%"},
	},
	},
	},
	}
	preHostHookOut := "pre-hook running with SKAFFOLD_KUBE_CONTEXT=context1,SKAFFOLD_NAMESPACES=np1,np2"
	postHostHookOut := "post-hook running with SKAFFOLD_KUBE_CONTEXT=context1,SKAFFOLD_NAMESPACES=np1,np2"
	namespaces := []string{"np1", "np2"}
	opts := NewRenderEnvOpts(testKubeContext, namespaces)
	runner := newRenderRunner(hooks, &namespaces, opts)
	t.Override(&kubernetesclient.Client, fakeKubernetesClient)
	var preOut, postOut bytes.Buffer
	err := runner.RunPreHooks(context.Background(), &preOut)
	t.CheckNoError(err)
	t.CheckContains(preHostHookOut, preOut.String())
	err = runner.RunPostHooks(context.Background(), &postOut)
	t.CheckNoError(err)
	t.CheckContains(postHostHookOut, postOut.String())
	})
	}

for lifecycle hooks

[codecov]

Codecov Report

Merging #9047 (e1b0d9a) into main (290280e) will decrease coverage by 7.14%.
Report is 1023 commits behind head on main.
The diff coverage is 49.04%.

@@            Coverage Diff             @@
##             main    #9047      +/-   ##
==========================================
- Coverage   70.48%   63.34%   -7.14%     
==========================================
  Files         515      626     +111     
  Lines       23150    32188    +9038     
==========================================
+ Hits        16317    20390    +4073     
- Misses       5776    10255    +4479     
- Partials     1057     1543     +486     

Files Changed	Coverage Δ
cmd/skaffold/app/cmd/completion.go	13.04% <0.00%> (-1.25%)	⬇️
cmd/skaffold/app/cmd/config/list.go	65.21% <ø> (ø)
cmd/skaffold/app/cmd/config/set.go	88.72% <ø> (ø)
cmd/skaffold/app/cmd/config/util.go	54.28% <ø> (ø)
cmd/skaffold/app/cmd/credits.go	100.00% <ø> (ø)
cmd/skaffold/app/cmd/credits/export.go	0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/deploy.go	40.90% <0.00%> (-12.94%)	⬇️
cmd/skaffold/app/cmd/generate_pipeline.go	60.00% <ø> (ø)
cmd/skaffold/app/cmd/inspect_modules.go	65.00% <0.00%> (ø)
cmd/skaffold/app/cmd/inspect_profiles.go	66.66% <0.00%> (ø)
... and 40 more

... and 430 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ericzzzzzzz]

A similar pr for render phase hook
#7785

[ar3s3ru]

Hey @ericzzzzzzz, thanks for the pointers, it does look very similar indeed. I'll take a stab at it later and ping you once I have something ready 🙏🏻

[ericzzzzzzz]

Hi @ar3s3ru Could you fix the linting issues? You can use make linters to test it locally

[ar3s3ru]

@ericzzzzzzz done, last commit should've fixed it.

[ar3s3ru]

Hey @ericzzzzzzz I see the unit tests are failing for linux. Seems to be a race condition non correlated to my changes.

How do you suggest to move forward? Open to anything to speed up merging this PR 🙏🏻