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

Conversion webhook #1302

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,10 @@ jobs:
version: v0.13.0
- name: Install Shipwright Build
run: |
make prepare-conversion
make install-controller-kind
kubectl -n shipwright-build rollout status deployment shipwright-build-controller --timeout=1m || true
kubectl -n shipwright-build rollout status deployment shipwright-build-webhook --timeout=1m || true
- name: Test
run: |
kubectl create namespace shp-e2e
Expand Down
10 changes: 10 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,12 @@ generate:
hack/install-controller-gen.sh
"$(CONTROLLER_GEN)" crd rbac:roleName=manager-role webhook paths="./..." output:crd:dir=deploy/crds

.PHONY: prepare-conversion
prepare-conversion:
hack/generate-cert.sh
hack/install-spruce.sh
hack/patch-crds-with-conversion.sh

.PHONY: verify-generate
verify-generate: generate
@hack/verify-generate.sh
Expand Down Expand Up @@ -152,6 +158,10 @@ endif
install-counterfeiter:
hack/install-counterfeiter.sh

.PHONY: install-spruce
install-spruce:
hack/install-spruce.sh

# Install golangci-lint via: go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
.PHONY: sanity-check
sanity-check:
Expand Down
109 changes: 109 additions & 0 deletions cmd/shipwright-build-webhook/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
// Copyright The Shipwright Contributors
//
// SPDX-License-Identifier: Apache-2.0

package main

import (
"context"
"crypto/tls"
"flag"
"fmt"
"net/http"
"os"
"path"
"runtime"
"time"

"github.com/shipwright-io/build/pkg/ctxlog"
"github.com/shipwright-io/build/pkg/webhook/conversion"
"github.com/shipwright-io/build/version"
"github.com/spf13/pflag"
"knative.dev/pkg/signals"
)

var (
versionGiven = flag.String("version", "devel", "Version of Shipwright webhook running")
)

func printVersion(ctx context.Context) {
ctxlog.Info(ctx, fmt.Sprintf("Shipwright Build Webhook Version: %s", version.Version))
ctxlog.Info(ctx, fmt.Sprintf("Go Version: %s", runtime.Version()))
ctxlog.Info(ctx, fmt.Sprintf("Go OS/Arch: %s/%s", runtime.GOOS, runtime.GOARCH))
Comment on lines +31 to +32
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this - is this data available/meaningful once the webhook is compiled into a binary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the version could be an "easy to get" indicator of which go version was used to build this image, assuming you are dealing with an specific CVE for go X version. We do this as well for the controller pod.

}

func main() {
// Add the zap logger flag set to the CLI. The flag set must
// be added before calling pflag.Parse().
pflag.CommandLine.AddGoFlagSet(ctxlog.CustomZapFlagSet())

// Add flags registered by imported packages (e.g. glog and
// controller-runtime)
pflag.CommandLine.AddGoFlagSet(flag.CommandLine)

pflag.Parse()

if err := Execute(); err != nil {
os.Exit(1)
}

}

func Execute() error {
l := ctxlog.NewLogger("shp-build-webhook")

ctx := ctxlog.NewParentContext(l)

version.SetVersion(*versionGiven)
printVersion(ctx)

mux := http.NewServeMux()
mux.HandleFunc("/health", health)
ctxlog.Info(ctx, "adding handlefunc() /health")

// convert endpoint handles ConversionReview API object serialized to JSON
mux.HandleFunc("/convert", conversion.CRDConvertHandler(ctx))
ctxlog.Info(ctx, "adding handlefunc() /convert")

server := &http.Server{
Addr: ":8443",
Handler: mux,
ReadHeaderTimeout: 32 * time.Second,
TLSConfig: &tls.Config{
MinVersion: tls.VersionTLS12,
CurvePreferences: []tls.CurveID{tls.CurveP256, tls.CurveP384, tls.X25519},
CipherSuites: []uint16{
tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,
},
Comment on lines +73 to +82
Copy link
Member

Choose a reason for hiding this comment

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

+1 on setting the min TLS version - I have seen scanners pick this up as a recommended practice. Are we seeing the same for the curve preferences and cipher suites?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the recommendation, following https://wiki.mozilla.org/Security/Server_Side_TLS per the Intermediate category. For example:

TLS 1.2 is the minimum supported protocol, as recommended by RFC 7525, PCI DSS, and others

The curve preferences are inline with the Intermediate category, the same as for the cipher suites.

},
}

go func() {
ctxlog.Info(ctx, "starting webhook server")
// blocking call, returns on error
if err := server.ListenAndServeTLS(path.Join("/etc/webhook/certs", "tls.crt"), path.Join("/etc/webhook/certs", "tls.key")); err != nil {
ctxlog.Error(ctx, err, "webhook server failed to start")
}
}()

stopCh := signals.SetupSignalHandler()
Copy link
Member

Choose a reason for hiding this comment

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

Not blocking: unsure if the signal handler should be set up earlier in the main function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

need to think more on this

sig := <-stopCh

l.Info("Shutting down server.", "signal", sig)
ctxlog.Info(ctx, "shutting down webhook server,", "signal:", sig)
if err := server.Shutdown(context.Background()); err != nil {
l.Error(err, "Failed to gracefully shutdown the server.")
return err
}
return nil

}

func health(resp http.ResponseWriter, _ *http.Request) {
resp.WriteHeader(http.StatusNoContent)
}
24 changes: 24 additions & 0 deletions deploy/201-role-webhook.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: shipwright-build-webhook
rules:
- apiGroups:
- ""
resources:
- pods
- events
- configmaps
- secrets
- limitranges
- namespaces
- services
verbs:
- '*'
- apiGroups:
- admissionregistration.k8s.io
- admissionregistration.k8s.io/v1beta1
resources:
- validatingwebhookconfigurations
verbs:
- '*'
14 changes: 14 additions & 0 deletions deploy/301-rolebinding-webhook.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: shipwright-build-webhook
namespace: shipwright-build
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: shipwright-build-webhook
subjects:
- kind: ServiceAccount
name: shipwright-build-webhook
namespace: shipwright-build
5 changes: 5 additions & 0 deletions deploy/401-serviceaccount-webhook.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
apiVersion: v1
kind: ServiceAccount
metadata:
name: shipwright-build-webhook
namespace: shipwright-build
12 changes: 12 additions & 0 deletions deploy/402-service-webhook.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
apiVersion: v1
kind: Service
metadata:
name: shp-build-webhook
namespace: shipwright-build
spec:
ports:
- name: https-webhook
port: 443
targetPort: 8443
selector:
name: shp-build-webhook
51 changes: 51 additions & 0 deletions deploy/700-deployment-webhook.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@

apiVersion: apps/v1
kind: Deployment
metadata:
name: shipwright-build-webhook
namespace: shipwright-build
labels:
app: shp-build-webhook
spec:
replicas: 1
strategy:
rollingUpdate:
maxSurge: 0
maxUnavailable: 1
type: RollingUpdate
selector:
matchLabels:
name: shp-build-webhook
template:
metadata:
name: shp-build-webhook
labels:
name: shp-build-webhook
spec:
securityContext:
runAsNonRoot: true
serviceAccountName: shipwright-build-webhook
containers:
- name: shp-build-webhook
image: ko://github.com/shipwright-io/build/cmd/shipwright-build-webhook
volumeMounts:
- name: webhook-certs
mountPath: /etc/webhook/certs
readOnly: true
ports:
- containerPort: 8443
name: https-port
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop:
- ALL
readOnlyRootFilesystem: true
runAsUser: 1000
runAsGroup: 1000
seccompProfile:
type: RuntimeDefault
volumes:
- name: webhook-certs
secret:
secretName: shipwright-build-webhook-cert
2 changes: 1 addition & 1 deletion deploy/crds/shipwright.io_buildruns.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12231,7 +12231,7 @@ spec:
required:
- spec
type: object
served: false
served: true
storage: false
subresources:
status: {}
2 changes: 1 addition & 1 deletion deploy/crds/shipwright.io_builds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4083,7 +4083,7 @@ spec:
required:
- spec
type: object
served: false
served: true
storage: false
subresources:
status: {}
2 changes: 1 addition & 1 deletion deploy/crds/shipwright.io_buildstrategies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4871,7 +4871,7 @@ spec:
description: BuildStrategyStatus defines the observed state of BuildStrategy
type: object
type: object
served: false
served: true
storage: false
subresources:
status: {}
2 changes: 1 addition & 1 deletion deploy/crds/shipwright.io_clusterbuildstrategies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4871,7 +4871,7 @@ spec:
description: BuildStrategyStatus defines the observed state of BuildStrategy
type: object
type: object
served: false
served: true
storage: false
subresources:
status: {}
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
github.com/go-logr/logr v1.2.4
github.com/golang-jwt/jwt/v4 v4.5.0
github.com/google/go-containerregistry v0.16.1
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822
github.com/onsi/ginkgo/v2 v2.12.0
github.com/onsi/gomega v1.27.10
github.com/prometheus/client_golang v1.16.0
Expand All @@ -17,6 +18,7 @@ require (
github.com/tektoncd/pipeline v0.44.0
go.uber.org/zap v1.25.0
k8s.io/api v0.25.6
k8s.io/apiextensions-apiserver v0.25.2
Copy link
Member

Choose a reason for hiding this comment

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

nit (not blocking): z-stream version skew for k8s library.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pls elaborate, I was not able to follow (keep in mind I'm sleepy :) )

k8s.io/apimachinery v0.25.6
k8s.io/client-go v0.25.6
k8s.io/code-generator v0.25.6
Expand Down Expand Up @@ -89,7 +91,6 @@ require (
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/opencontainers/go-digest v1.0.0 // indirect
github.com/opencontainers/image-spec v1.1.0-rc3 // indirect
github.com/pjbgf/sha1cd v0.3.0 // indirect
Expand Down
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,7 @@ honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9
k8s.io/api v0.25.6 h1:LwDY2H6kD/3R8TekJYYaJWOdekNdXDO44eVpX6sNtJA=
k8s.io/api v0.25.6/go.mod h1:bVp01KUcl8VUHFBTJMOknWNo7XvR0cMbeTTuFg1zCUs=
k8s.io/apiextensions-apiserver v0.25.2 h1:8uOQX17RE7XL02ngtnh3TgifY7EhekpK+/piwzQNnBo=
k8s.io/apiextensions-apiserver v0.25.2/go.mod h1:iRwwRDlWPfaHhuBfQ0WMa5skdQfrE18QXJaJvIDLvE8=
k8s.io/apimachinery v0.25.6 h1:r6KIF2AHwLqFfZ0LcOA3I11SF62YZK83dxj1fn14NOQ=
k8s.io/apimachinery v0.25.6/go.mod h1:1S2i1QHkmxc8+EZCIxe/fX5hpldVXk4gvnJInMEb8D4=
k8s.io/client-go v0.25.6 h1:CHxACHi0DijmlYyUR7ooZoXnD5P8jYLgBHcxp775x/U=
Expand Down
12 changes: 12 additions & 0 deletions hack/customization/conversion_webhook_block.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
spec:
conversion:
strategy: Webhook
webhook:
clientConfig:
caBundle: CA_BUNDLE
service:
namespace: shipwright-build
name: shp-build-webhook
path: /convert
conversionReviewVersions:
- v1