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

Make unit testing easier/make examples work #3

Open
munnerz opened this issue May 2, 2019 · 11 comments
Open

Make unit testing easier/make examples work #3

munnerz opened this issue May 2, 2019 · 11 comments
Labels
priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.

Comments

@munnerz
Copy link
Member

munnerz commented May 2, 2019

Right now if you run go test within this repository, the tests will fail.

They fail for a few reasons:

  • We don't provide any way to fetch the test binaries (i.e. kube-apiserver & etcd)
  • The actual sample will always panic on Present/CleanUp
  • We don't actually set any TXT records anywhere when Present is called, meaning the test suite will always fail

(1) can be fixed by adding a fetch-binaries.sh script (probably taken straight from sigs.k8s.io/controller-tools).

(2) and (3) can be most easily fixed by embedding a 'fake dns server' into the webhook, which will listen for DNS queries and serve TXT records accordingly. This is how cert-manager itself handles tests for the RFC2136 DNS01 provider, and there is a reusable library that we can use to build it easily: https://github.com/jetstack/cert-manager/tree/master/test/acme/dns/server

@cmoulliard
Copy link

cmoulliard commented May 29, 2019

The documented command with the README file is also wrong as the domain name should been defined as such

$ TEST_ZONE_NAME=example.com. go test .

and not as

$ TEST_ZONE_NAME=example.com go test .

@cmoulliard
Copy link

If we add the content of this bash script and call it before to run the test

./scripts/fetch-test-binaries.sh 
using tools
fetching tools
x kubebuilder/
x kubebuilder/bin/
x kubebuilder/bin/etcd
x kubebuilder/bin/kubectl
x kubebuilder/bin/kube-apiserver
kubebuilder tools (etcd, kubectl, kube-apiserver)used to perform local tests installed under ./_out/kubebuilder/bin/

Bash script

#!/usr/bin/env bash

set -e

#hack_dir=$(dirname ${BASH_SOURCE})
#source ${hack_dir}/common.sh

k8s_version=1.14.1
goarch=amd64
goos="unknown"

if [[ "$OSTYPE" == "linux-gnu" ]]; then
  goos="linux"
elif [[ "$OSTYPE" == "darwin"* ]]; then
  goos="darwin"
fi

if [[ "$goos" == "unknown" ]]; then
  echo "OS '$OSTYPE' not supported. Aborting." >&2
  exit 1
fi

tmp_root=./_out
kb_root_dir=$tmp_root/kubebuilder

# Turn colors in this script off by setting the NO_COLOR variable in your
# environment to any value:
#
# $ NO_COLOR=1 test.sh
NO_COLOR=${NO_COLOR:-""}
if [ -z "$NO_COLOR" ]; then
  header=$'\e[1;33m'
  reset=$'\e[0m'
else
  header=''
  reset=''
fi

function header_text {
  echo "$header$*$reset"
}

# fetch k8s API gen tools and make it available under kb_root_dir/bin.
function fetch_kb_tools {
  header_text "fetching tools"
  mkdir -p $tmp_root
  kb_tools_archive_name="kubebuilder-tools-$k8s_version-$goos-$goarch.tar.gz"
  kb_tools_download_url="https://storage.googleapis.com/kubebuilder-tools/$kb_tools_archive_name"

  kb_tools_archive_path="$tmp_root/$kb_tools_archive_name"
  if [ ! -f $kb_tools_archive_path ]; then
    curl -sL ${kb_tools_download_url} -o "$kb_tools_archive_path"
  fi
  tar -zvxf "$kb_tools_archive_path" -C "$tmp_root/"
}

header_text "using tools"
fetch_kb_tools

header_text "kubebuilder tools (etcd, kubectl, kube-apiserver)used to perform local tests installed under $tmp_root/kubebuilder/bin/"
exit 0

and set the binary path here

package main

import (
	"os"
	"testing"

	"github.com/jetstack/cert-manager/test/acme/dns"
)

var (
	zone = os.Getenv("TEST_ZONE_NAME")
	kubeBuilderBinPath = "./_out/kubebuilder/bin"
)

func TestRunsSuite(t *testing.T) {
	// The manifest path should contain a file named config.json that is a
	// snippet of valid configuration that should be included on the
	// ChallengeRequest passed as part of the test cases.

	fixture := dns.NewFixture(&customDNSProviderSolver{},
	    dns.SetBinariesPath(kubeBuilderBinPath),
		dns.SetResolvedZone(zone),
		dns.SetAllowAmbientCredentials(false),
		dns.SetManifestPath("testdata/my-custom-solver"),
	)

	fixture.RunConformance(t)
}

then the test can work locally

=== RUN   TestRunsSuite/Conformance/Basic/PresentRecord
I0529 10:07:54.590584   96706 request.go:947] Request Body: {"kind":"Namespace","apiVersion":"v1","metadata":{"name":"basic-present-record","creationTimestamp":null},"spec":{},"status":{}}
I0529 10:07:54.590693   96706 round_trippers.go:419] curl -k -v -XPOST  -H "Content-Type: application/json" -H "User-Agent: ___main_test_go/v0.0.0 (darwin/amd64) kubernetes/$Format" -H "Accept: application/json, */*" 'http://127.0.0.1:50095/api/v1/namespaces'
I0529 10:07:54.593889   96706 round_trippers.go:438] POST http://127.0.0.1:50095/api/v1/namespaces 201 Created in 3 milliseconds
I0529 10:07:54.593936   96706 round_trippers.go:444] Response Headers:
I0529 10:07:54.593945   96706 round_trippers.go:447]     Content-Type: application/json
I0529 10:07:54.593952   96706 round_trippers.go:447]     Date: Wed, 29 May 2019 08:07:54 GMT
I0529 10:07:54.593963   96706 round_trippers.go:447]     Content-Length: 311
I0529 10:07:54.595697   96706 request.go:947] Response Body: {"kind":"Namespace","apiVersion":"v1","metadata":{"name":"basic-present-record","selfLink":"/api/v1/namespaces/basic-present-record","uid":"dca85b60-81e8-11e9-ac2c-4c32758b7e5f","resourceVersion":"40","creationTimestamp":"2019-05-29T08:07:54Z"},"spec":{"finalizers":["kubernetes"]},"status":{"phase":"Active"}}
Decoded configuration {}I0529 10:07:58.648438   96706 wait.go:269] Searching fqdn "cert-manager-dns01-tests.example.com." using seed nameservers [8.8.8.8:53]
I0529 10:07:58.834931   96706 wait.go:336] Returning discovered zone record "example.com." for fqdn "cert-manager-dns01-tests.example.com."
I0529 10:07:58.877248   96706 wait.go:287] Returning authoritative nameservers [a.iana-servers.net., b.iana-servers.net.]
I0529 10:07:59.000405   96706 wait.go:120] Looking up TXT records for "cert-manager-dns01-tests.example.com."
I0529 10:08:01.651670   96706 wait.go:269] Searching fqdn "cert-manager-dns01-tests.example.com." using seed nameservers [8.8.8.8:53]
I0529 10:08:01.651718   96706 wait.go:300] Returning cached zone record "example.com." for fqdn "cert-manager-dns01-tests.example.com."
I0529 10:08:01.696203   96706 wait.go:287] Returning authoritative nameservers [a.iana-servers.net., b.iana-servers.net.]
I0529 10:08:01.817908   96706 wait.go:120] Looking up TXT records for "cert-manager-dns01-tests.example.com."
...

@cmoulliard
Copy link

Can we expect an update on the ticket ? @munnerz

@mansona
Copy link

mansona commented Feb 10, 2021

Hey folks 👋 I tried my hand at the (1) problem you mentioned @munnerz and now I'm struggling with even knowing what to do next 😂

You mention that the test suite doesn't try to set any records anywhere, but I'm wondering if it were possible to actually have this test run "live" with a test account with the provider you're trying to integrate with. I know it would obviously be better to mock everything if you're trying to setup CI/CD, but I'm currently just trying to get the project working and care a little bit less about best practices 🙈 😂

@jakexks
Copy link
Member

jakexks commented Feb 24, 2021

Hello! - relatively new cert-manager project member here reading some very old issues. I bumped the version of everything and made an example package that passes the text fixture.

@mansona if you set UseAuthoritative to true in the test fixture I believe the test suite will actually try to resolve the test domains while running.

@mansona
Copy link

mansona commented Feb 24, 2021

ooo thanks @jakexks 👍 is the example package you talk about public? are you able to share a link?

@jakexks
Copy link
Member

jakexks commented Feb 24, 2021

It's in the main repo, so pulling the latest changes and typing make test will run it

https://github.com/cert-manager/webhook-example/tree/master/example

@mansona
Copy link

mansona commented Apr 20, 2021

Ok so looking at this again, yes it looks like the new example works (as in the tests pass) but it now looks very different from the previous example 😂 I'm trying to use the examples and some info from forks of this repo to piece together a webhook handler even though I don't actually know how to write go 🙃

You mention I should set UseAuthorative to true but I don't see this defined or used anywhere 🤔 what do you mean by this?

@mansona
Copy link

mansona commented Apr 22, 2021

So I finally was able to push through and get the initial version of my webhook handler implemented 🎉 I'm going to try and get it tested in a real kubernetes deployment at some point over the next week: https://github.com/mansona/cert-manager-webhook-netlify

I ended up not using your updated code @jakexks and more-so leaned on some other examples like https://github.com/NetWatcher/cert-manager-webhook-godaddy and https://github.com/bwolf/cert-manager-webhook-gandi

I am interested in helping make this process a bit easier for people with a bit more guidance so if there is anything that I can try to assist with that please let me know. I will say that I'm not sure if my difficulty was caused by any lack of guidance from the example, or just my extreme lack of knowledge of golang 🙈 This is the first thing that I have ever written in golang

@toksdotdev
Copy link

toksdotdev commented Jul 12, 2021

@mansona Please were you able to get the netlify DNS webhook tested in Kubernetes? I'm currently experiencing the same issue, and seems there's very little documentation on how to handle the deployment.

@wallrj
Copy link
Member

wallrj commented Feb 10, 2022

The make test target does at least download the Kubebuilder binaries now

jsundqvist pushed a commit to jsundqvist/webhook-example that referenced this issue Feb 21, 2024
jsundqvist pushed a commit to jsundqvist/webhook-example that referenced this issue Feb 21, 2024
@inteon inteon added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Development

No branches or pull requests

7 participants