From c1c75a8f22b148efb624b9c35f05c6ff57851d47 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 5 Oct 2020 14:53:50 -0500 Subject: [PATCH 1/6] Add kube_cert_agent_image value to main ytt template. This needs to be overridden for Tilt usage, since the main image referenced by Tilt isn't actually pullable. Signed-off-by: Matt Moyer --- deploy/deployment.yaml | 4 ++++ deploy/values.yaml | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/deploy/deployment.yaml b/deploy/deployment.yaml index 2a1d5e9e..b946ca7c 100644 --- a/deploy/deployment.yaml +++ b/deploy/deployment.yaml @@ -40,11 +40,15 @@ data: apiService: (@= data.values.app_name + "-api" @) kubeCertAgent: namePrefix: (@= data.values.app_name + "-kube-cert-agent-" @) + (@ if data.values.kube_cert_agent_image: @) + image: (@= data.values.kube_cert_agent_image @) + (@ else: @) (@ if data.values.image_digest: @) image: (@= data.values.image_repo + "@" + data.values.image_digest @) (@ else: @) image: (@= data.values.image_repo + ":" + data.values.image_tag @) (@ end @) + (@ end @) (@ if data.values.image_pull_dockerconfigjson: @) imagePullSecrets: - image-pull-secret diff --git a/deploy/values.yaml b/deploy/values.yaml index b579a212..0a25f7ee 100644 --- a/deploy/values.yaml +++ b/deploy/values.yaml @@ -15,6 +15,11 @@ image_repo: docker.io/getpinniped/pinniped-server image_digest: #! e.g. sha256:f3c4fdfd3ef865d4b97a1fd295d94acc3f0c654c46b6f27ffad5cf80216903c8 image_tag: latest +#! Optionally specify a different image for the "kube-cert-agent" pod which is scheduled +#! on the control plane. This image needs only to include `sleep` and `cat` binaries. +#! By default, the same image specified for image_repo/image_digest/image_tag will be re-used. +kube_cert_agent_image: + #! Specifies a secret to be used when pulling the above container image. #! Can be used when the above image_repo is a private registry. #! Typically the value would be the output of: kubectl create secret docker-registry x --docker-server=https://example.io --docker-username="USERNAME" --docker-password="PASSWORD" --dry-run=client -o json | jq -r '.data[".dockerconfigjson"]' From 01153dcb9daa8fc9b5850ad9e7caae9100d33325 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Fri, 2 Oct 2020 16:01:28 -0500 Subject: [PATCH 2/6] Iterate on pull request template. - Moves the "front matter" to a Markdown comment so you don't necessarily have to delete it. - Reduces a little bit of boilerplate (this is a bit subjective). - Tweaks some formatting (also subjective). - Describe what happens when you use "Fixes [...]". - Tweak the release note block so it should be easier to parse out automatically (using the same syntax as Kubernetes). Signed-off-by: Matt Moyer --- .github/pull_request_template.md | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 89c22a00..3dfa1f06 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,3 +1,4 @@ + --- Please delete this line and all lines above this line before submitting the PR. Thanks! -- + -**Things to consider while reviewing this PR** + -**Suggested release note for the first release which contains this PR** +**Release note**: + + +```release-note ``` -release-note here -``` From b0a4ae13c58374d997bfa9bb799d5ead7396ae48 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Tue, 8 Sep 2020 11:29:34 -0500 Subject: [PATCH 3/6] Add Tilt-based local dev workflow. Signed-off-by: Matt Moyer --- .gitignore | 3 + .pre-commit-config.yaml | 2 +- CONTRIBUTING.md | 39 ++++- cmd/pinniped-server/main.go | 8 +- hack/lib/tilt/Tiltfile | 97 +++++++++++ .../tilt/local-user-authenticator.Dockerfile | 14 ++ hack/lib/tilt/pinniped.Dockerfile | 14 ++ .../tilt_modules/docker_build_sub/Tiltfile | 44 +++++ hack/lib/tilt/tilt_modules/extensions.json | 16 ++ .../tilt_modules/restart_process/Tiltfile | 78 +++++++++ hack/prepare-for-integration-tests.sh | 163 ++++++++++-------- hack/tilt-down.sh | 9 + hack/tilt-up.sh | 9 + 13 files changed, 416 insertions(+), 80 deletions(-) create mode 100644 hack/lib/tilt/Tiltfile create mode 100644 hack/lib/tilt/local-user-authenticator.Dockerfile create mode 100644 hack/lib/tilt/pinniped.Dockerfile create mode 100644 hack/lib/tilt/tilt_modules/docker_build_sub/Tiltfile create mode 100644 hack/lib/tilt/tilt_modules/extensions.json create mode 100644 hack/lib/tilt/tilt_modules/restart_process/Tiltfile create mode 100755 hack/tilt-down.sh create mode 100755 hack/tilt-up.sh diff --git a/.gitignore b/.gitignore index 2aaba826..3a5c3201 100644 --- a/.gitignore +++ b/.gitignore @@ -16,3 +16,6 @@ # goland .idea + +# Intermediate files used by Tilt +/hack/lib/tilt/build diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f4712345..50581ddd 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,4 +1,4 @@ -exclude: '^generated/' +exclude: '^(generated|hack/lib/tilt/tilt_modules)/' repos: - repo: git://github.com/pre-commit/pre-commit-hooks rev: v3.2.0 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 191520fd..29e4d7ce 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -93,12 +93,41 @@ docker build . ### Running Integration Tests -```bash -./hack/prepare-for-integration-tests.sh && source /tmp/integration-test-env && go test -v -count 1 ./test/... -``` +1. Install dependencies: -The `./hack/prepare-for-integration-tests.sh` script will create a local -[`kind`](https://kind.sigs.k8s.io/) cluster on which the integration tests will run. + - [`kind`](https://kind.sigs.k8s.io/docs/user/quick-start) + - [`tilt`](https://docs.tilt.dev/install.html) + - [`ytt`](https://carvel.dev/#getting-started) + - [`kubectl`](https://kubernetes.io/docs/tasks/tools/install-kubectl/) + + On macOS, these tools can be installed with [Homebrew](https://brew.sh/): + + ```bash + brew install kind tilt-dev/tap/tilt k14s/tap/ytt kubectl + ``` + +1. Create a local Kubernetes cluster using `kind`: + + ```bash + kind create cluster --image kindest/node:v1.18.8 + ``` + +1. Install Pinniped and supporting dependencies using `tilt`: + + ```bash + ./hack/tilt-up.sh + ``` + + Tilt will continue running and live-updating the Pinniped deployment whenever the code changes. + +1. Run the Pinniped integration tests: + + ```bash + source ./hack/lib/tilt/integration-test.env && go test -v -count 1 ./test/integration + ``` + +To uninstall the test environment, run `./hack/tilt-down.sh`. +To destroy the local Kubernetes cluster, run `kind delete cluster`. ### Observing Tests on the Continuous Integration Environment diff --git a/cmd/pinniped-server/main.go b/cmd/pinniped-server/main.go index 5741a696..42293796 100644 --- a/cmd/pinniped-server/main.go +++ b/cmd/pinniped-server/main.go @@ -5,6 +5,7 @@ package main import ( "os" + "time" genericapiserver "k8s.io/apiserver/pkg/server" "k8s.io/client-go/pkg/version" @@ -19,7 +20,12 @@ func main() { logs.InitLogs() defer logs.FlushLogs() - klog.Infof("Running %s at %#v", rest.DefaultKubernetesUserAgent(), version.Get()) + // Dump out the time since compile (mostly useful for benchmarking our local development cycle latency). + var timeSinceCompile time.Duration + if buildDate, err := time.Parse(time.RFC3339, version.Get().BuildDate); err == nil { + timeSinceCompile = time.Since(buildDate).Round(time.Second) + } + klog.Infof("Running %s at %#v (%s since build)", rest.DefaultKubernetesUserAgent(), version.Get(), timeSinceCompile) ctx := genericapiserver.SetupSignalContext() diff --git a/hack/lib/tilt/Tiltfile b/hack/lib/tilt/Tiltfile new file mode 100644 index 00000000..0a7372ef --- /dev/null +++ b/hack/lib/tilt/Tiltfile @@ -0,0 +1,97 @@ +load('ext://restart_process', 'docker_build_with_restart') +disable_snapshots() +analytics_settings(False) +update_settings(max_parallel_updates=8) +os.putenv('CGO_ENABLED', '0') +os.putenv('GOOS', 'linux') +os.putenv('GOARCH', 'amd64') +os.putenv('CGO_ENABLED', '0') +os.putenv('KUBE_GIT_VERSION', 'v0.0.0') + +# Compile all of our ./cmd/... binaries. +local_resource( + 'compile', + 'cd ../../../ && mkdir -p ./hack/lib/tilt/build && go build -v -ldflags "$(hack/get-ldflags.sh)" -o ./hack/lib/tilt/build ./cmd/...', + deps=['../../../cmd', '../../../internal', '../../../pkg', '../../../generated'], +) + +# Build a container image for local-user-authenticator, with live-update enabled. +docker_build_with_restart('image/local-user-auth', '.', + dockerfile='local-user-authenticator.Dockerfile', + entrypoint=['/usr/local/bin/local-user-authenticator'], + live_update=[sync('./build/local-user-authenticator', '/usr/local/bin/local-user-authenticator')], + only=['./build/local-user-authenticator'], +) + +# Render the local-user-authenticator installation manifest using ytt. +k8s_yaml(local([ + 'ytt', + '--file', '../../../deploy-local-user-authenticator', + '--data-value', 'image_repo=image/local-user-auth', + '--data-value', 'image_tag=tilt-dev', +])) + +# Collect all the deployed local-user-authenticator resources under a "local-user-auth" resource tab. +k8s_resource( + workload='local-user-authenticator', + new_name='local-user-auth', + objects=[ + 'local-user-authenticator:namespace', + 'local-user-authenticator:serviceaccount', + 'local-user-authenticator:role', + 'local-user-authenticator:rolebinding', + ], +) + +# Build a container image for the Pinniped server, with live-update enabled. +docker_build_with_restart('image/pinniped', '.', + dockerfile='pinniped.Dockerfile', + entrypoint=['/usr/local/bin/pinniped-server'], + live_update=[sync('./build/pinniped-server', '/usr/local/bin/pinniped-server')], + only=['./build/pinniped-server'], +) + +# Render the Pinniped server installation manifest using ytt. +k8s_yaml(local([ + 'sh', '-c', + 'ytt --file ../../../deploy ' + + '--data-value namespace=integration ' + + '--data-value image_repo=image/pinniped ' + + '--data-value image_tag=tilt-dev ' + + '--data-value kube_cert_agent_image=debian:10.5-slim ' + + '--data-value discovery_url=$(TERM=dumb kubectl cluster-info | awk \'/Kubernetes master/ {print $NF}\') ' + + '--data-value-yaml replicas=1' +])) + +# Collect all the deployed local-user-authenticator resources under a "deploy/pinniped" resource tab. +k8s_resource( + workload='pinniped', + objects=[ + 'integration:namespace', + 'credentialissuerconfigs.config.pinniped.dev:customresourcedefinition', + 'webhookidentityproviders.idp.pinniped.dev:customresourcedefinition', + 'pinniped:serviceaccount', + 'pinniped-aggregated-api-server:role', + 'pinniped-kube-system-pod-read:role', + 'pinniped-cluster-info-lister-watcher:role', + 'pinniped-aggregated-api-server:clusterrole', + 'pinniped-create-token-credential-requests:clusterrole', + 'pinniped-aggregated-api-server:rolebinding', + 'pinniped-kube-system-pod-read:rolebinding', + 'pinniped-extension-apiserver-authentication-reader:rolebinding', + 'pinniped-cluster-info-lister-watcher:rolebinding', + 'pinniped-aggregated-api-server:clusterrolebinding', + 'pinniped-create-token-credential-requests:clusterrolebinding', + 'pinniped:clusterrolebinding', + 'pinniped-config:configmap', + 'v1alpha1.login.pinniped.dev:apiservice', + ], +) + +# Collect environment variables needed to run our integration test suite. +local_resource( + 'test-env', + 'TILT_MODE=yes ../../prepare-for-integration-tests.sh', + resource_deps=['local-user-auth', 'pinniped'], + deps=['../../prepare-for-integration-tests.sh'], +) diff --git a/hack/lib/tilt/local-user-authenticator.Dockerfile b/hack/lib/tilt/local-user-authenticator.Dockerfile new file mode 100644 index 00000000..1853ccd8 --- /dev/null +++ b/hack/lib/tilt/local-user-authenticator.Dockerfile @@ -0,0 +1,14 @@ +# Copyright 2020 VMware, Inc. +# SPDX-License-Identifier: Apache-2.0 + +# Use a runtime image based on Debian slim +FROM debian:10.5-slim + +# Copy the binary which was built outside the container. +COPY build/local-user-authenticator /usr/local/bin/local-user-authenticator + +# Document the port +EXPOSE 443 + +# Set the entrypoint +ENTRYPOINT ["/usr/local/bin/local-user-authenticator"] diff --git a/hack/lib/tilt/pinniped.Dockerfile b/hack/lib/tilt/pinniped.Dockerfile new file mode 100644 index 00000000..8bbc4d13 --- /dev/null +++ b/hack/lib/tilt/pinniped.Dockerfile @@ -0,0 +1,14 @@ +# Copyright 2020 VMware, Inc. +# SPDX-License-Identifier: Apache-2.0 + +# Use a runtime image based on Debian slim +FROM debian:10.5-slim + +# Copy the binary which was built outside the container. +COPY build/pinniped-server /usr/local/bin/pinniped-server + +# Document the port +EXPOSE 443 + +# Set the entrypoint +ENTRYPOINT ["/usr/local/bin/pinniped-server"] diff --git a/hack/lib/tilt/tilt_modules/docker_build_sub/Tiltfile b/hack/lib/tilt/tilt_modules/docker_build_sub/Tiltfile new file mode 100644 index 00000000..ccdd1a30 --- /dev/null +++ b/hack/lib/tilt/tilt_modules/docker_build_sub/Tiltfile @@ -0,0 +1,44 @@ +def docker_build_sub(ref, context, extra_cmds, child_context=None, base_suffix='-tilt_docker_build_sub_base', live_update=[], **kwargs): + """ + Substitutes in a docker image with extra Dockerfile commands. + + This allows you to easily customize your docker build for your dev environment without changing your prod Dockerfile. + + This works by: + 1. Renaming the original image to, e.g. "myimage-base" + 2. Creating a new image named, e.g. "myimage" that starts with "FROM myimage-base" + 3. Adding whatever extra stuff you want + + Examples: + ``` + # load the extension + load("ext://docker_build_sub", "docker_build_sub") + + # ensure you have vim installed when running in dev, so you can + # shell into the box and look at files + docker_build_sub('myimage', '.', extra_cmds=["apt-get install vim"]) + + # use live_update to sync files from outside your docker context + docker_build_sub('foo', 'foo', child_context='bar', + extra_cmds=['ADD . /bar'], + live_update=[ + sync('foo', '/foo'), + sync('bar', '/bar'), + ] + ) + ``` + + This function supports all the normal `docker_build` arguments. See [docker_build API docs](https://docs.tilt.dev/api.html#api.docker_build) for arguments not mentioned here.. + + Args: + context (str): The directory in which to build the parent (original) image. If child_context is not set, also the directory in which to build the new child image. + extra_cmds (List[str]): Any extra Dockerfile commands you want to run when building the image. + child_context (str): The directory in which to build the new child image. If unset (None), defaults to the parent image's context. + base_suffix (str): The suffix to append to the parent (original) image's name so that the new child image can take the original name. This is mostly ignorable, and just here in case the default generates a conflict for you. + """ + if not child_context: + child_context = context + base_ref = '%s-base' % ref + docker_build(base_ref, context, **kwargs) + df = '\n'.join(['FROM %s' % base_ref] + extra_cmds) + docker_build(ref, child_context, dockerfile_contents=df, live_update=live_update, **kwargs) diff --git a/hack/lib/tilt/tilt_modules/extensions.json b/hack/lib/tilt/tilt_modules/extensions.json new file mode 100644 index 00000000..0094be30 --- /dev/null +++ b/hack/lib/tilt/tilt_modules/extensions.json @@ -0,0 +1,16 @@ +{ + "Extensions": [ + { + "Name": "restart_process", + "GitCommitHash": "b8df6f5f3368ced855da56e002027a3bd1a61bdf", + "ExtensionRegistry": "https://github.com/tilt-dev/tilt-extensions", + "TimeFetched": "2020-09-03T23:04:40.167635-05:00" + }, + { + "Name": "docker_build_sub", + "GitCommitHash": "b8df6f5f3368ced855da56e002027a3bd1a61bdf", + "ExtensionRegistry": "https://github.com/tilt-dev/tilt-extensions", + "TimeFetched": "2020-09-04T18:01:24.795509-05:00" + } + ] +} \ No newline at end of file diff --git a/hack/lib/tilt/tilt_modules/restart_process/Tiltfile b/hack/lib/tilt/tilt_modules/restart_process/Tiltfile new file mode 100644 index 00000000..fc43809f --- /dev/null +++ b/hack/lib/tilt/tilt_modules/restart_process/Tiltfile @@ -0,0 +1,78 @@ +RESTART_FILE = '/.restart-proc' +TYPE_RESTART_CONTAINER_STEP = 'live_update_restart_container_step' + +KWARGS_BLACKLIST = [ + # since we'll be passing `dockerfile_contents` when building the + # child image, remove any kwargs that might conflict + 'dockerfile', 'dockerfile_contents', + + # 'target' isn't relevant to our child build--if we pass this arg, + # Docker will just fail to find the specified stage and error out + 'target', +] + +def docker_build_with_restart(ref, context, entrypoint, live_update, + base_suffix='-tilt_docker_build_with_restart_base', restart_file=RESTART_FILE, **kwargs): + """Wrap a docker_build call and its associated live_update steps so that the last step + of any live update is to rerun the given entrypoint. + + + Args: + ref: name for this image (e.g. 'myproj/backend' or 'myregistry/myproj/backend'); as the parameter of the same name in docker_build + context: path to use as the Docker build context; as the parameter of the same name in docker_build + entrypoint: the command to be (re-)executed when the container starts or when a live_update is run + live_update: set of steps for updating a running container; as the parameter of the same name in docker_build + base_suffix: suffix for naming the base image, applied as {ref}{base_suffix} + restart_file: file that Tilt will update during a live_update to signal the entrypoint to rerun + **kwargs: will be passed to the underlying `docker_build` call + """ + + # first, validate the given live_update steps + if len(live_update) == 0: + fail("`docker_build_with_restart` requires at least one live_update step") + for step in live_update: + if type(step) == TYPE_RESTART_CONTAINER_STEP: + fail("`docker_build_with_restart` is not compatible with live_update step: "+ + "`restart_container()` (this extension is meant to REPLACE restart_container() )") + + # rename the original image to make it a base image and declare a docker_build for it + base_ref = '{}{}'.format(ref, base_suffix) + docker_build(base_ref, context, **kwargs) + + # declare a new docker build that adds a static binary of tilt-restart-wrapper + # (which makes use of `entr` to watch files and restart processes) to the user's image + df = ''' + FROM tiltdev/restart-helper:2020-07-16 as restart-helper + + FROM {} + USER root + RUN ["touch", "{}"] + COPY --from=restart-helper /tilt-restart-wrapper / + COPY --from=restart-helper /entr / + '''.format(base_ref, restart_file) + + # Clean kwargs for building the child image (which builds on user's specified + # image and copies in Tilt's restart wrapper). In practice, this means removing + # kwargs that were relevant to building the user's specified image but are NOT + # relevant to building the child image / may conflict with args we specifically + # pass for the child image. + cleaned_kwargs = {k: v for k, v in kwargs.items() if k not in KWARGS_BLACKLIST} + + # Change the entrypoint to use `tilt-restart-wrapper`. + # `tilt-restart-wrapper` makes use of `entr` (https://github.com/eradman/entr/) to + # re-execute $entrypoint whenever $restart_file changes + if type(entrypoint) == type(""): + entrypoint_with_entr = ["/tilt-restart-wrapper", "--watch_file={}".format(restart_file), "sh", "-c", entrypoint] + elif type(entrypoint) == type([]): + entrypoint_with_entr = ["/tilt-restart-wrapper", "--watch_file={}".format(restart_file)] + entrypoint + else: + fail("`entrypoint` must be a string or list of strings: got {}".format(type(entrypoint))) + + # last live_update step should always be to modify $restart_file, which + # triggers the process wrapper to rerun $entrypoint + # NB: write `date` instead of just `touch`ing because `entr` doesn't respond + # to timestamp changes, only writes (see https://github.com/eradman/entr/issues/32) + live_update = live_update + [run('date > {}'.format(restart_file))] + + docker_build(ref, context, entrypoint=entrypoint_with_entr, dockerfile_contents=df, + live_update=live_update, **cleaned_kwargs) diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index 8340f2e4..e4f3bd0f 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -9,6 +9,14 @@ set -euo pipefail # # Helper functions # +TILT_MODE=${TILT_MODE:-no} +function tilt_mode() { + if [[ "$TILT_MODE" == "yes" ]]; then + return 0 + fi + return 1 +} + function log_note() { GREEN='\033[0;32m' NC='\033[0m' @@ -94,67 +102,71 @@ if [ "$(kubectl version --client=true --short | cut -d '.' -f 2)" -lt 18 ]; then exit 1 fi -# -# Setup kind and build the app -# -log_note "Checking for running kind clusters..." -if ! kind get clusters | grep -q -e '^kind$'; then - log_note "Creating a kind cluster..." - kind create cluster -else - if ! kubectl cluster-info | grep master | grep -q 127.0.0.1; then - log_error "Seems like your kubeconfig is not targeting a local cluster." - log_error "Exiting to avoid accidentally running tests against a real cluster." - exit 1 - fi -fi - -registry="docker.io" -repo="test/build" -registry_repo="$registry/$repo" -tag=$(uuidgen) # always a new tag to force K8s to reload the image on redeploy - -if [[ "$skip_build" == "yes" ]]; then - most_recent_tag=$(docker images "$repo" --format "{{.Tag}}" | head -1) - if [[ -n "$most_recent_tag" ]]; then - tag="$most_recent_tag" - do_build=no +if ! tilt_mode; then + # + # Setup kind and build the app + # + log_note "Checking for running kind clusters..." + if ! kind get clusters | grep -q -e '^kind$'; then + log_note "Creating a kind cluster..." + kind create cluster + else + if ! kubectl cluster-info | grep master | grep -q 127.0.0.1; then + log_error "Seems like your kubeconfig is not targeting a local cluster." + log_error "Exiting to avoid accidentally running tests against a real cluster." + exit 1 + fi + fi + + registry="docker.io" + repo="test/build" + registry_repo="$registry/$repo" + tag=$(uuidgen) # always a new tag to force K8s to reload the image on redeploy + + if [[ "$skip_build" == "yes" ]]; then + most_recent_tag=$(docker images "$repo" --format "{{.Tag}}" | head -1) + if [[ -n "$most_recent_tag" ]]; then + tag="$most_recent_tag" + do_build=no + else + # Oops, there was no previous build. Need to build anyway. + do_build=yes + fi else - # Oops, there was no previous build. Need to build anyway. do_build=yes fi -else - do_build=yes + + registry_repo_tag="${registry_repo}:${tag}" + + if [[ "$do_build" == "yes" ]]; then + # Rebuild the code + log_note "Docker building the app..." + docker build . --tag "$registry_repo_tag" + fi + + # Load it into the cluster + log_note "Loading the app's container image into the kind cluster..." + kind load docker-image "$registry_repo_tag" + + manifest=/tmp/manifest.yaml + + # + # Deploy local-user-authenticator + # + pushd deploy-local-user-authenticator >/dev/null + + log_note "Deploying the local-user-authenticator app to the cluster..." + ytt --file . \ + --data-value "image_repo=$registry_repo" \ + --data-value "image_tag=$tag" >"$manifest" + + kubectl apply --dry-run=client -f "$manifest" # Validate manifest schema. + kapp deploy --yes --app local-user-authenticator --diff-changes --file "$manifest" + + popd >/dev/null + fi -registry_repo_tag="${registry_repo}:${tag}" - -if [[ "$do_build" == "yes" ]]; then - # Rebuild the code - log_note "Docker building the app..." - docker build . --tag "$registry_repo_tag" -fi - -# Load it into the cluster -log_note "Loading the app's container image into the kind cluster..." -kind load docker-image "$registry_repo_tag" - -manifest=/tmp/manifest.yaml - -# -# Deploy local-user-authenticator -# -pushd deploy-local-user-authenticator >/dev/null - -log_note "Deploying the local-user-authenticator app to the cluster..." -ytt --file . \ - --data-value "image_repo=$registry_repo" \ - --data-value "image_tag=$tag" >"$manifest" - -kubectl apply --dry-run=client -f "$manifest" # Validate manifest schema. -kapp deploy --yes --app local-user-authenticator --diff-changes --file "$manifest" - -popd >/dev/null test_username="test-username" test_groups="test-group-0,test-group-1" @@ -180,22 +192,24 @@ webhook_url="https://local-user-authenticator.local-user-authenticator.svc/authe webhook_ca_bundle="$(kubectl get secret local-user-authenticator-tls-serving-certificate --namespace local-user-authenticator -o 'jsonpath={.data.caCertificate}')" discovery_url="$(TERM=dumb kubectl cluster-info | awk '/Kubernetes master/ {print $NF}')" -# -# Deploy Pinniped -# -pushd deploy >/dev/null +if ! tilt_mode; then + # + # Deploy Pinniped + # + pushd deploy >/dev/null -log_note "Deploying the Pinniped app to the cluster..." -ytt --file . \ - --data-value "app_name=$app_name" \ - --data-value "namespace=$namespace" \ - --data-value "image_repo=$registry_repo" \ - --data-value "image_tag=$tag" \ - --data-value "discovery_url=$discovery_url" >"$manifest" + log_note "Deploying the Pinniped app to the cluster..." + ytt --file . \ + --data-value "app_name=$app_name" \ + --data-value "namespace=$namespace" \ + --data-value "image_repo=$registry_repo" \ + --data-value "image_tag=$tag" \ + --data-value "discovery_url=$discovery_url" >"$manifest" -kapp deploy --yes --app "$app_name" --diff-changes --file "$manifest" + kapp deploy --yes --app "$app_name" --diff-changes --file "$manifest" -popd >/dev/null + popd >/dev/null +fi # # Create the environment file @@ -233,7 +247,10 @@ log_note log_note 'Want to run integration tests in GoLand? Copy/paste this "Environment" value for GoLand run configurations:' log_note " ${goland_vars}PINNIPED_CLUSTER_CAPABILITY_FILE=${kind_capabilities_file}" log_note -log_note "You can rerun this script to redeploy local production code changes while you are working." -log_note -log_note "To delete the deployments, run 'kapp delete -a local-user-authenticator -y && kapp delete -a pinniped -y'." -log_note "When you're finished, use 'kind delete cluster' to tear down the cluster." + +if ! tilt_mode; then + log_note "You can rerun this script to redeploy local production code changes while you are working." + log_note + log_note "To delete the deployments, run 'kapp delete -a local-user-authenticator -y && kapp delete -a pinniped -y'." + log_note "When you're finished, use 'kind delete cluster' to tear down the cluster." +fi diff --git a/hack/tilt-down.sh b/hack/tilt-down.sh new file mode 100755 index 00000000..95992122 --- /dev/null +++ b/hack/tilt-down.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash + +# Copyright 2020 the Pinniped contributors. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +set -euo pipefail +ROOT="$( cd "$( dirname "${BASH_SOURCE[0]}" )/.." && pwd )" +cd "${ROOT}" +exec tilt down -f ./hack/lib/tilt/Tiltfile diff --git a/hack/tilt-up.sh b/hack/tilt-up.sh new file mode 100755 index 00000000..acbf5a3f --- /dev/null +++ b/hack/tilt-up.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash + +# Copyright 2020 the Pinniped contributors. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +set -euo pipefail +ROOT="$( cd "$( dirname "${BASH_SOURCE[0]}" )/.." && pwd )" +cd "${ROOT}" +exec tilt up -f ./hack/lib/tilt/Tiltfile --stream From 38501ff763bbf15e01a4aae29010efe9ba5358d4 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 5 Oct 2020 19:09:51 -0500 Subject: [PATCH 4/6] Add initial "pinniped alpha login oidc" partial implementation. Signed-off-by: Matt Moyer --- cmd/pinniped/cmd/alpha.go | 22 +++ cmd/pinniped/cmd/cobra_util.go | 15 ++ cmd/pinniped/cmd/cobra_util_test.go | 21 +++ cmd/pinniped/cmd/get_kubeconfig.go | 5 +- cmd/pinniped/cmd/login.go | 21 +++ cmd/pinniped/cmd/login_oidc.go | 126 ++++++++++++++++ cmd/pinniped/cmd/login_oidc_test.go | 220 ++++++++++++++++++++++++++++ go.mod | 4 + go.sum | 6 + internal/oidc/pkce/pkce.go | 45 ++++++ internal/oidc/pkce/pkce_test.go | 42 ++++++ internal/oidc/state/state.go | 37 +++++ internal/oidc/state/state_test.go | 25 ++++ 13 files changed, 585 insertions(+), 4 deletions(-) create mode 100644 cmd/pinniped/cmd/alpha.go create mode 100644 cmd/pinniped/cmd/cobra_util.go create mode 100644 cmd/pinniped/cmd/cobra_util_test.go create mode 100644 cmd/pinniped/cmd/login.go create mode 100644 cmd/pinniped/cmd/login_oidc.go create mode 100644 cmd/pinniped/cmd/login_oidc_test.go create mode 100644 internal/oidc/pkce/pkce.go create mode 100644 internal/oidc/pkce/pkce_test.go create mode 100644 internal/oidc/state/state.go create mode 100644 internal/oidc/state/state_test.go diff --git a/cmd/pinniped/cmd/alpha.go b/cmd/pinniped/cmd/alpha.go new file mode 100644 index 00000000..db27150f --- /dev/null +++ b/cmd/pinniped/cmd/alpha.go @@ -0,0 +1,22 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "github.com/spf13/cobra" +) + +//nolint: gochecknoglobals +var alphaCmd = &cobra.Command{ + Use: "alpha", + Short: "alpha", + Long: "alpha subcommands (syntax or flags are still subject to change)", + SilenceUsage: true, // do not print usage message when commands fail + Hidden: true, +} + +//nolint: gochecknoinits +func init() { + rootCmd.AddCommand(alphaCmd) +} diff --git a/cmd/pinniped/cmd/cobra_util.go b/cmd/pinniped/cmd/cobra_util.go new file mode 100644 index 00000000..9b153a72 --- /dev/null +++ b/cmd/pinniped/cmd/cobra_util.go @@ -0,0 +1,15 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import "github.com/spf13/cobra" + +// mustMarkRequired marks the given flags as required on the provided cobra.Command. If any of the names are wrong, it panics. +func mustMarkRequired(cmd *cobra.Command, flags ...string) { + for _, flag := range flags { + if err := cmd.MarkFlagRequired(flag); err != nil { + panic(err) + } + } +} diff --git a/cmd/pinniped/cmd/cobra_util_test.go b/cmd/pinniped/cmd/cobra_util_test.go new file mode 100644 index 00000000..b44e8550 --- /dev/null +++ b/cmd/pinniped/cmd/cobra_util_test.go @@ -0,0 +1,21 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" +) + +func TestMustMarkRequired(t *testing.T) { + require.NotPanics(t, func() { mustMarkRequired(&cobra.Command{}) }) + require.NotPanics(t, func() { + cmd := &cobra.Command{} + cmd.Flags().String("known-flag", "", "") + mustMarkRequired(cmd, "known-flag") + }) + require.Panics(t, func() { mustMarkRequired(&cobra.Command{}, "unknown-flag") }) +} diff --git a/cmd/pinniped/cmd/get_kubeconfig.go b/cmd/pinniped/cmd/get_kubeconfig.go index 0660c556..8ed99b0d 100644 --- a/cmd/pinniped/cmd/get_kubeconfig.go +++ b/cmd/pinniped/cmd/get_kubeconfig.go @@ -85,15 +85,12 @@ func (c *getKubeConfigCommand) Command() *cobra.Command { `), } cmd.Flags().StringVar(&c.flags.token, "token", "", "Credential to include in the resulting kubeconfig output (Required)") - err := cmd.MarkFlagRequired("token") - if err != nil { - panic(err) - } cmd.Flags().StringVar(&c.flags.kubeconfig, "kubeconfig", c.flags.kubeconfig, "Path to the kubeconfig file") cmd.Flags().StringVar(&c.flags.contextOverride, "kubeconfig-context", c.flags.contextOverride, "Kubeconfig context override") cmd.Flags().StringVar(&c.flags.namespace, "pinniped-namespace", c.flags.namespace, "Namespace in which Pinniped was installed") cmd.Flags().StringVar(&c.flags.idpType, "idp-type", c.flags.idpType, "Identity provider type (e.g., 'webhook')") cmd.Flags().StringVar(&c.flags.idpName, "idp-name", c.flags.idpType, "Identity provider name") + mustMarkRequired(cmd, "token") return cmd } diff --git a/cmd/pinniped/cmd/login.go b/cmd/pinniped/cmd/login.go new file mode 100644 index 00000000..2c0ad082 --- /dev/null +++ b/cmd/pinniped/cmd/login.go @@ -0,0 +1,21 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "github.com/spf13/cobra" +) + +//nolint: gochecknoglobals +var loginCmd = &cobra.Command{ + Use: "login", + Short: "login", + Long: "Login to a Pinniped server", + SilenceUsage: true, // do not print usage message when commands fail +} + +//nolint: gochecknoinits +func init() { + alphaCmd.AddCommand(loginCmd) +} diff --git a/cmd/pinniped/cmd/login_oidc.go b/cmd/pinniped/cmd/login_oidc.go new file mode 100644 index 00000000..e9287d43 --- /dev/null +++ b/cmd/pinniped/cmd/login_oidc.go @@ -0,0 +1,126 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "fmt" + + "github.com/coreos/go-oidc" + "github.com/pkg/browser" + "github.com/spf13/cobra" + "golang.org/x/oauth2" + + "go.pinniped.dev/internal/here" + "go.pinniped.dev/internal/oidc/pkce" + "go.pinniped.dev/internal/oidc/state" +) + +//nolint: gochecknoinits +func init() { + loginCmd.AddCommand((&oidcLoginParams{ + generateState: state.Generate, + generatePKCE: pkce.Generate, + openURL: browser.OpenURL, + }).cmd()) +} + +type oidcLoginParams struct { + // These parameters capture CLI flags. + issuer string + clientID string + listenPort uint16 + scopes []string + skipBrowser bool + usePKCE bool + debugAuthCode bool + + // These parameters capture dependencies that we want to mock during testing. + generateState func() (state.State, error) + generatePKCE func() (pkce.Code, error) + openURL func(string) error +} + +func (o *oidcLoginParams) cmd() *cobra.Command { + cmd := cobra.Command{ + Args: cobra.NoArgs, + Use: "oidc --issuer ISSUER --client-id CLIENT_ID", + Short: "Login using an OpenID Connect provider", + RunE: o.runE, + SilenceUsage: true, + } + cmd.Flags().StringVar(&o.issuer, "issuer", "", "OpenID Connect issuer URL.") + cmd.Flags().StringVar(&o.clientID, "client-id", "", "OpenID Connect client ID.") + cmd.Flags().Uint16Var(&o.listenPort, "listen-port", 48095, "TCP port for localhost listener (authorization code flow only).") + cmd.Flags().StringSliceVar(&o.scopes, "scopes", []string{"offline_access", "openid", "email", "profile"}, "OIDC scopes to request during login.") + cmd.Flags().BoolVar(&o.skipBrowser, "skip-browser", false, "Skip opening the browser (just print the URL).") + cmd.Flags().BoolVar(&o.usePKCE, "use-pkce", true, "Use Proof Key for Code Exchange (RFC 7636) during login.") + mustMarkRequired(&cmd, "issuer", "client-id") + + // TODO: temporary + cmd.Flags().BoolVar(&o.debugAuthCode, "debug-auth-code-exchange", true, "Debug the authorization code exchange (temporary).") + _ = cmd.Flags().MarkHidden("debug-auth-code-exchange") + + return &cmd +} + +func (o *oidcLoginParams) runE(cmd *cobra.Command, args []string) error { + metadata, err := oidc.NewProvider(cmd.Context(), o.issuer) + if err != nil { + return fmt.Errorf("could not perform OIDC discovery for %q: %w", o.issuer, err) + } + + cfg := oauth2.Config{ + ClientID: o.clientID, + Endpoint: metadata.Endpoint(), + RedirectURL: fmt.Sprintf("http://localhost:%d/callback", o.listenPort), + Scopes: o.scopes, + } + + authCodeOptions := []oauth2.AuthCodeOption{oauth2.AccessTypeOffline} + + stateParam, err := o.generateState() + if err != nil { + return fmt.Errorf("could not generate OIDC state parameter: %w", err) + } + + var pkceCode pkce.Code + if o.usePKCE { + pkceCode, err = o.generatePKCE() + if err != nil { + return fmt.Errorf("could not generate OIDC PKCE parameter: %w", err) + } + authCodeOptions = append(authCodeOptions, pkceCode.Challenge(), pkceCode.Method()) + } + + // If --skip-browser was passed, override the default browser open function with a Printf() call. + openURL := o.openURL + if o.skipBrowser { + openURL = func(s string) error { + cmd.PrintErr("Please log in: ", s, "\n") + return nil + } + } + + authorizeURL := cfg.AuthCodeURL(stateParam.String(), authCodeOptions...) + if err := openURL(authorizeURL); err != nil { + return fmt.Errorf("could not open browser (run again with --skip-browser?): %w", err) + } + + // TODO: this temporary so we can complete the auth code exchange manually + + if o.debugAuthCode { + cmd.PrintErr(here.Docf(` + DEBUG INFO: + Token URL: %s + State: %s + PKCE: %s + `, + cfg.Endpoint.TokenURL, + stateParam, + pkceCode.Verifier(), + )) + } + + return nil +} diff --git a/cmd/pinniped/cmd/login_oidc_test.go b/cmd/pinniped/cmd/login_oidc_test.go new file mode 100644 index 00000000..9258cc30 --- /dev/null +++ b/cmd/pinniped/cmd/login_oidc_test.go @@ -0,0 +1,220 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "bytes" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "regexp" + "strings" + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" + + "go.pinniped.dev/internal/here" + "go.pinniped.dev/internal/oidc/pkce" + "go.pinniped.dev/internal/oidc/state" +) + +func TestLoginOIDCCommand(t *testing.T) { + t.Parallel() + tests := []struct { + name string + args []string + wantError bool + wantStdout string + wantStderr string + }{ + { + name: "help flag passed", + args: []string{"--help"}, + wantStdout: here.Doc(` + Login using an OpenID Connect provider + + Usage: + oidc --issuer ISSUER --client-id CLIENT_ID [flags] + + Flags: + --client-id string OpenID Connect client ID. + -h, --help help for oidc + --issuer string OpenID Connect issuer URL. + --listen-port uint16 TCP port for localhost listener (authorization code flow only). (default 48095) + --scopes strings OIDC scopes to request during login. (default [offline_access,openid,email,profile]) + --skip-browser Skip opening the browser (just print the URL). + --use-pkce Use Proof Key for Code Exchange (RFC 7636) during login. (default true) + `), + }, + { + name: "missing required flags", + args: []string{}, + wantError: true, + wantStdout: here.Doc(` + Error: required flag(s) "client-id", "issuer" not set + `), + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + cmd := (&oidcLoginParams{}).cmd() + require.NotNil(t, cmd) + + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs(tt.args) + err := cmd.Execute() + if tt.wantError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantStdout, stdout.String(), "unexpected stdout") + require.Equal(t, tt.wantStderr, stderr.String(), "unexpected stderr") + }) + } +} + +func TestOIDCLoginRunE(t *testing.T) { + t.Parallel() + + // Start a server that returns 500 errors. + brokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + })) + t.Cleanup(brokenServer.Close) + + // Start a server that returns successfully. + var validResponse string + validServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(validResponse)) + })) + t.Cleanup(validServer.Close) + validResponse = strings.ReplaceAll(here.Docf(` + { + "issuer": "${ISSUER}", + "authorization_endpoint": "${ISSUER}/auth", + "token_endpoint": "${ISSUER}/token", + "jwks_uri": "${ISSUER}/keys", + "userinfo_endpoint": "${ISSUER}/userinfo", + "grant_types_supported": ["authorization_code","refresh_token"], + "response_types_supported": ["code"], + "id_token_signing_alg_values_supported": ["RS256"], + "scopes_supported": ["openid","email","groups","profile","offline_access"], + "token_endpoint_auth_methods_supported": ["client_secret_basic"], + "claims_supported": ["aud","email","email_verified","exp","iat","iss","locale","name","sub"] + } + `), "${ISSUER}", validServer.URL) + validServerURL, err := url.Parse(validServer.URL) + require.NoError(t, err) + + tests := []struct { + name string + params oidcLoginParams + wantError string + wantStdout string + wantStderr string + wantStderrAuthURL func(*testing.T, *url.URL) + }{ + { + name: "broken discovery", + params: oidcLoginParams{ + issuer: brokenServer.URL, + }, + wantError: fmt.Sprintf("could not perform OIDC discovery for %q: 500 Internal Server Error: Internal Server Error\n", brokenServer.URL), + }, + { + name: "broken state generation", + params: oidcLoginParams{ + issuer: validServer.URL, + generateState: func() (state.State, error) { return "", fmt.Errorf("some error generating a state value") }, + }, + wantError: "could not generate OIDC state parameter: some error generating a state value", + }, + { + name: "broken PKCE generation", + params: oidcLoginParams{ + issuer: validServer.URL, + generateState: func() (state.State, error) { return "test-state", nil }, + usePKCE: true, + generatePKCE: func() (pkce.Code, error) { return "", fmt.Errorf("some error generating a PKCE code") }, + }, + wantError: "could not generate OIDC PKCE parameter: some error generating a PKCE code", + }, + { + name: "broken browser open", + params: oidcLoginParams{ + issuer: validServer.URL, + generateState: func() (state.State, error) { return "test-state", nil }, + usePKCE: true, + generatePKCE: func() (pkce.Code, error) { return "test-pkce", nil }, + openURL: func(_ string) error { return fmt.Errorf("some browser open error") }, + }, + wantError: "could not open browser (run again with --skip-browser?): some browser open error", + }, + { + name: "success without PKCE", + params: oidcLoginParams{ + issuer: validServer.URL, + clientID: "test-client-id", + generateState: func() (state.State, error) { return "test-state", nil }, + usePKCE: false, + listenPort: 12345, + skipBrowser: true, + }, + wantStderrAuthURL: func(t *testing.T, actual *url.URL) { + require.Equal(t, validServerURL.Host, actual.Host) + require.Equal(t, "/auth", actual.Path) + require.Equal(t, "", actual.Fragment) + require.Equal(t, url.Values{ + "access_type": []string{"offline"}, + "client_id": []string{"test-client-id"}, + "redirect_uri": []string{"http://localhost:12345/callback"}, + "response_type": []string{"code"}, + "state": []string{"test-state"}, + }, actual.Query()) + }, + wantStderr: "Please log in: \n", + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + var stdout, stderr bytes.Buffer + cmd := cobra.Command{RunE: tt.params.runE, SilenceUsage: true, SilenceErrors: true} + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + err := cmd.Execute() + if tt.wantError != "" { + require.EqualError(t, err, tt.wantError) + } else { + require.NoError(t, err) + } + + if tt.wantStderrAuthURL != nil { + var urls []string + redacted := regexp.MustCompile(`http://\S+`).ReplaceAllStringFunc(stderr.String(), func(url string) string { + urls = append(urls, url) + return "" + }) + require.Lenf(t, urls, 1, "expected to find authorization URL in stderr:\n%s", stderr.String()) + authURL, err := url.Parse(urls[0]) + require.NoError(t, err, "invalid authorization URL") + tt.wantStderrAuthURL(t, authURL) + + // Replace the stderr buffer with the redacted version. + stderr.Reset() + stderr.WriteString(redacted) + } + + require.Equal(t, tt.wantStdout, stdout.String(), "unexpected stdout") + require.Equal(t, tt.wantStderr, stderr.String(), "unexpected stderr") + }) + } +} diff --git a/go.mod b/go.mod index cf6f6a2e..a3f63b6b 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.14 require ( github.com/MakeNowJust/heredoc/v2 v2.0.1 + github.com/coreos/go-oidc v2.2.1+incompatible github.com/davecgh/go-spew v1.1.1 github.com/ghodss/yaml v1.0.0 github.com/go-logr/logr v0.2.1 @@ -11,6 +12,8 @@ require ( github.com/golang/mock v1.4.4 github.com/golangci/golangci-lint v1.31.0 github.com/google/go-cmp v0.5.2 + github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4 + github.com/pkg/errors v0.9.1 github.com/sclevine/spec v1.4.0 github.com/spf13/cobra v1.0.0 github.com/spf13/pflag v1.0.5 @@ -18,6 +21,7 @@ require ( go.pinniped.dev/generated/1.19/apis v0.0.0-00010101000000-000000000000 go.pinniped.dev/generated/1.19/client v0.0.0-00010101000000-000000000000 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 + golang.org/x/oauth2 v0.0.0-20191202225959-858c2ad4c8b6 k8s.io/api v0.19.2 k8s.io/apimachinery v0.19.2 k8s.io/apiserver v0.19.2 diff --git a/go.sum b/go.sum index 200cf57f..dae855b9 100644 --- a/go.sum +++ b/go.sum @@ -88,6 +88,8 @@ github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkE github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= github.com/coreos/go-oidc v2.1.0+incompatible/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc= +github.com/coreos/go-oidc v2.2.1+incompatible h1:mh48q/BqXqgjVHpy2ZY7WnWAbenxRjsz9N1i1YxjHAk= +github.com/coreos/go-oidc v2.2.1+incompatible/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc= github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk= github.com/coreos/go-semver v0.3.0 h1:wkHLiw0WNATZnSG7epLsujiMCgPAc9xhjJ4tgnAxmfM= github.com/coreos/go-semver v0.3.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk= @@ -450,6 +452,8 @@ github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/9 github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= github.com/phayes/checkstyle v0.0.0-20170904204023-bfd46e6a821d h1:CdDQnGF8Nq9ocOS/xlSptM1N3BbrA6/kmaep5ggwaIA= github.com/phayes/checkstyle v0.0.0-20170904204023-bfd46e6a821d/go.mod h1:3OzsM7FXDQlpCiw2j81fOmAwQLnZnLGXVKUzeKQXIAw= +github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4 h1:49lOXmGaUpV9Fz3gd7TFZY106KVlPVa5jcYD1gaQf98= +github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4/go.mod h1:4OwLy04Bl9Ef3GJJCoec+30X3LQs/0/m4HFRt/2LUSA= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= @@ -457,6 +461,7 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/posener/complete v1.1.1/go.mod h1:em0nMJCgc9GFtwrmVmEMR/ZL6WyhyjMBndrE9hABlRI= +github.com/pquerna/cachecontrol v0.0.0-20171018203845-0dec1b30a021 h1:0XM1XL/OFFJjXsYXlG30spTkV/E9+gmd5GD1w2HE8xM= github.com/pquerna/cachecontrol v0.0.0-20171018203845-0dec1b30a021/go.mod h1:prYjPmNq4d1NPVmpShWobRqXY3q7Vp+80DqgxxUrUIA= github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw= github.com/prometheus/client_golang v0.9.3/go.mod h1:/TN21ttK/J9q6uSwhBd54HahCDft0ttaMvbicHlPoso= @@ -835,6 +840,7 @@ gopkg.in/ini.v1 v1.51.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/natefinch/lumberjack.v2 v2.0.0 h1:1Lc07Kr7qY4U2YPouBjpCLxpiyxIVoxqXgkXLknAOE8= gopkg.in/natefinch/lumberjack.v2 v2.0.0/go.mod h1:l0ndWWf7gzL7RNwBG7wST/UCcT4T24xpD6X8LsfU/+k= gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo= +gopkg.in/square/go-jose.v2 v2.2.2 h1:orlkJ3myw8CN1nVQHBFfloD+L3egixIa4FvUP6RosSA= gopkg.in/square/go-jose.v2 v2.2.2/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= diff --git a/internal/oidc/pkce/pkce.go b/internal/oidc/pkce/pkce.go new file mode 100644 index 00000000..309b3d4d --- /dev/null +++ b/internal/oidc/pkce/pkce.go @@ -0,0 +1,45 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package pkce + +import ( + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "io" + + "github.com/pkg/errors" + "golang.org/x/oauth2" +) + +// Generate generates a new random PKCE code. +func Generate() (Code, error) { return generate(rand.Reader) } + +func generate(rand io.Reader) (Code, error) { + var buf [32]byte + if _, err := io.ReadFull(rand, buf[:]); err != nil { + return "", errors.WithMessage(err, "could not generate PKCE code") + } + return Code(hex.EncodeToString(buf[:])), nil +} + +// Code implements the basic options required for RFC 7636: Proof Key for Code Exchange (PKCE). +type Code string + +// Challenge returns the OAuth2 auth code parameter for sending the PKCE code challenge. +func (p *Code) Challenge() oauth2.AuthCodeOption { + b := sha256.Sum256([]byte(*p)) + return oauth2.SetAuthURLParam("code_challenge", base64.RawURLEncoding.EncodeToString(b[:])) +} + +// Method returns the OAuth2 auth code parameter for sending the PKCE code challenge method. +func (p *Code) Method() oauth2.AuthCodeOption { + return oauth2.SetAuthURLParam("code_challenge_method", "S256") +} + +// Verifier returns the OAuth2 auth code parameter for sending the PKCE code verifier. +func (p *Code) Verifier() oauth2.AuthCodeOption { + return oauth2.SetAuthURLParam("code_verifier", string(*p)) +} diff --git a/internal/oidc/pkce/pkce_test.go b/internal/oidc/pkce/pkce_test.go new file mode 100644 index 00000000..be611378 --- /dev/null +++ b/internal/oidc/pkce/pkce_test.go @@ -0,0 +1,42 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package pkce + +import ( + "bytes" + "encoding/base64" + "net/url" + "testing" + + "golang.org/x/oauth2" + + "github.com/stretchr/testify/require" +) + +func TestPKCE(t *testing.T) { + p, err := Generate() + require.NoError(t, err) + + cfg := oauth2.Config{} + authCodeURL, err := url.Parse(cfg.AuthCodeURL("", p.Challenge(), p.Method())) + require.NoError(t, err) + + // The code_challenge must be 256 bits (sha256) encoded as unpadded urlsafe base64. + chal, err := base64.RawURLEncoding.DecodeString(authCodeURL.Query().Get("code_challenge")) + require.NoError(t, err) + require.Len(t, chal, 32) + + // The code_challenge_method must be a fixed value. + require.Equal(t, "S256", authCodeURL.Query().Get("code_challenge_method")) + + // The code_verifier param should be 64 hex characters. + verifyURL, err := url.Parse(cfg.AuthCodeURL("", p.Verifier())) + require.NoError(t, err) + require.Regexp(t, `\A[0-9a-f]{64}\z`, verifyURL.Query().Get("code_verifier")) + + var empty bytes.Buffer + p, err = generate(&empty) + require.EqualError(t, err, "could not generate PKCE code: EOF") + require.Empty(t, p) +} diff --git a/internal/oidc/state/state.go b/internal/oidc/state/state.go new file mode 100644 index 00000000..7d70e51b --- /dev/null +++ b/internal/oidc/state/state.go @@ -0,0 +1,37 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package state + +import ( + "crypto/rand" + "crypto/subtle" + "encoding/hex" + "io" + + "github.com/pkg/errors" +) + +// Generate generates a new random state parameter of an appropriate size. +func Generate() (State, error) { return generate(rand.Reader) } + +func generate(rand io.Reader) (State, error) { + var buf [16]byte + if _, err := io.ReadFull(rand, buf[:]); err != nil { + return "", errors.WithMessage(err, "could not generate random state") + } + return State(hex.EncodeToString(buf[:])), nil +} + +// State implements some utilities for working with OAuth2 state parameters. +type State string + +// String returns the string encoding of this state value. +func (s *State) String() string { + return string(*s) +} + +// Validate the returned state (from a callback parameter). Returns true iff the state is valid. +func (s *State) Valid(returnedState string) bool { + return subtle.ConstantTimeCompare([]byte(returnedState), []byte(*s)) == 1 +} diff --git a/internal/oidc/state/state_test.go b/internal/oidc/state/state_test.go new file mode 100644 index 00000000..ff181839 --- /dev/null +++ b/internal/oidc/state/state_test.go @@ -0,0 +1,25 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package state + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestState(t *testing.T) { + s, err := Generate() + require.NoError(t, err) + require.Len(t, s, 32) + require.Len(t, s.String(), 32) + require.True(t, s.Valid(string(s))) + require.False(t, s.Valid(string(s)+"x")) + + var empty bytes.Buffer + s, err = generate(&empty) + require.EqualError(t, err, "could not generate random state: EOF") + require.Empty(t, s) +} From 6f8f99e49b8254b4f942e5682d2e5d0fb8355a31 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Tue, 6 Oct 2020 16:35:58 -0700 Subject: [PATCH 5/6] Add demo screencast and do some cleanup in demo.md - Note that this avoids committing the demo screencast file to our git history because it is 5.76 MB. We won't want to need to download that content on every `git clone`. - Instead the file is hosted by GitHub's CDN --- doc/demo.md | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/doc/demo.md b/doc/demo.md index 88b72839..a555d38c 100644 --- a/doc/demo.md +++ b/doc/demo.md @@ -9,17 +9,15 @@ 1. An identity provider of a type supported by Pinniped as described in [doc/architecture.md](../doc/architecture.md). - Don't have an identity provider of a type supported by Pinniped handy? - Start by installing `local-user-authenticator` on the same cluster where you would like to try Pinniped + Don't have an identity provider of a type supported by Pinniped handy? No problem, there is a demo identity provider + available. Start by installing local-user-authenticator on the same cluster where you would like to try Pinniped by following the directions in [deploy-local-user-authenticator/README.md](../deploy-local-user-authenticator/README.md). See below for an example of deploying this on kind. 1. A kubeconfig where the current context points to the cluster and has admin-like privileges on that cluster. -## Steps - -### Overview +## Overview Installing and trying Pinniped on any cluster will consist of the following general steps. See the next section below for a more specific example of installing onto a local kind cluster, including the exact commands to use for that case. @@ -29,7 +27,23 @@ for a more specific example of installing onto a local kind cluster, including t 1. Generate a kubeconfig using the Pinniped CLI. Run `pinniped get-kubeconfig --help` for more information. 1. Run `kubectl` commands using the generated kubeconfig. Pinniped will automatically be used for authentication during those commands. -### Steps to Deploy the Latest Release on kind Using local-user-authenticator as the Identity Provider +## Example of Deploying on kind + +[kind](https://kind.sigs.k8s.io) is a tool for creating and managing Kubernetes clusters on your local machine +which uses Docker containers as the cluster's "nodes". This is a convenient way to try out Pinniped on a local +non-production cluster. + +The following steps will deploy the latest release of Pinniped on kind using the local-user-authenticator component +as the identity provider. + + +

+Pinniped Installation Demo +

1. Install the tools required for the following steps. @@ -65,7 +79,8 @@ for a more specific example of installing onto a local kind cluster, including t pinniped_version=v0.2.0 ``` -1. Deploy the `local-user-authenticator` app. +1. Deploy the local-user-authenticator app. This is a demo identity provider. In production, you would use your + real identity provider, and therefore would not need to deploy or configure local-user-authenticator. ```bash kubectl apply -f https://github.com/vmware-tanzu/pinniped/releases/download/$pinniped_version/install-local-user-authenticator.yaml @@ -76,7 +91,7 @@ for a more specific example of installing onto a local kind cluster, including t see [deploy-local-user-authenticator/README.md](../deploy-local-user-authenticator/README.md) for instructions on how to deploy using `ytt`. -1. Create a test user. +1. Create a test user named `pinny-the-seal` in the local-user-authenticator identity provider. ```bash kubectl create secret generic pinny-the-seal \ @@ -85,7 +100,7 @@ for a more specific example of installing onto a local kind cluster, including t --from-literal=passwordHash=$(htpasswd -nbBC 10 x password123 | sed -e "s/^x://") ``` -1. Fetch the auto-generated CA bundle for the `local-user-authenticator`'s HTTP TLS endpoint. +1. Fetch the auto-generated CA bundle for the local-user-authenticator's HTTP TLS endpoint. ```bash kubectl get secret local-user-authenticator-tls-serving-certificate --namespace local-user-authenticator \ @@ -103,7 +118,7 @@ for a more specific example of installing onto a local kind cluster, including t If you would prefer to customize the available options, please see [deploy/README.md](../deploy/README.md) for instructions on how to deploy using `ytt`. -1. Create a `WebhookIdentityProvider` object to configure Pinniped to authenticate using `local-user-authenticator`. +1. Create a `WebhookIdentityProvider` object to configure Pinniped to authenticate using local-user-authenticator. ```bash cat < Date: Tue, 6 Oct 2020 17:11:08 -0700 Subject: [PATCH 6/6] Update the file used as the demo screencast New version of the file was created by @danjahner --- doc/demo.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/demo.md b/doc/demo.md index a555d38c..52d9e766 100644 --- a/doc/demo.md +++ b/doc/demo.md @@ -39,7 +39,7 @@ as the identity provider.

Pinniped Installation Demo