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.

Sign up for GitHub

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

Improve test case #5

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Improve test case #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6294ce1
Ignore idea and _out directory
cmoulliard May 29, 2019
3bb6f6c
Add missing dot end of the DNS name
cmoulliard May 29, 2019
9b4aa49
Update dependencies. To be controlled
cmoulliard May 29, 2019
f96df3b
Uncomment email and ApiSecretRef
cmoulliard May 29, 2019
296fbde
Add script to fetch kubebuilder binaries
cmoulliard May 29, 2019
060ffb3
Add DNS server. Set additional vars
cmoulliard May 29, 2019
7621c67
Remove non used vars. Add setUseAuthoritative(false) to avoid the err…
cmoulliard May 29, 2019
7415d9a
Added local DNS server. Signed-off-by: Charles Moulliard <cmoulliard@…
cmoulliard Jun 3, 2019
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
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

# Output of the go coverage tool, specifically when used with LiteIDE
*.out
_out
.idea/

# Ignore the built binary
cert-manager-webhook-example
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ An example Go test file has been provided in [main_test.go]().
You can run the test suite with:

```bash
$ TEST_ZONE_NAME=example.com go test .
$ TEST_ZONE_NAME=snowdrop.me. go test .
cmoulliard marked this conversation as resolved.
Show resolved Hide resolved
```

The example file has a number of areas you must fill in and replace with your
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ github.com/globalsign/mgo v0.0.0-20181015135952-eeefdecb41b8/go.mod h1:xkRDCp4j0
github.com/go-ini/ini v1.42.0/go.mod h1:ByCAeIL28uOIIG0E3PJtZPDL8WnHpFKFOtgjp+3Ies8=
github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE=
github.com/go-logr/logr v0.1.0 h1:M1Tv3VzNlEHg6uyACnRdtrploV2P7wZqH8BoQMtz0cg=
github.com/go-logr/logr v0.1.0/go.mod h1:ixOQHD9gLJUVQQ2ZOR7zLEifBX6tGkNJF4QyIY7sIas=
github.com/go-logr/zapr v0.1.1/go.mod h1:tabnROwaDl0UNxkVeFRbY8bwB37GwRv0P8lg6aAiEnk=
github.com/go-openapi/analysis v0.0.0-20180825180245-b006789cd277/go.mod h1:k70tL6pCuVxPJOHXQ+wIac1FUrvNkHolPie/cLEU6hI=
Expand Down Expand Up @@ -448,6 +449,7 @@ k8s.io/klog v0.0.0-20190306015804-8e90cee79f82 h1:SHucoAy7lRb+w5oC/hbXyZg+zX+Wft
k8s.io/klog v0.0.0-20190306015804-8e90cee79f82/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk=
k8s.io/klog v0.2.0 h1:0ElL0OHzF3N+OhoJTL0uca20SxtYt4X4+bzHeqrB83c=
k8s.io/klog v0.2.0/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk=
k8s.io/kube-aggregator v0.0.0-20190222095010-0b78038fe9e5 h1:ft+D1QY+Dwtek4Q9g3NFxRHm9qFjRwJmEkholIeF+NM=
k8s.io/kube-aggregator v0.0.0-20190222095010-0b78038fe9e5/go.mod h1:8sbzT4QQKDEmSCIbfqjV0sd97GpUT7A4W626sBiYJmU=
k8s.io/kube-openapi v0.0.0-20190228160746-b3a7cee44a30 h1:TRb4wNWoBVrH9plmkp2q86FIDppkbrEXdXlxU3a3BMI=
k8s.io/kube-openapi v0.0.0-20190228160746-b3a7cee44a30/go.mod h1:BXM9ceUBTj2QnfH2MK1odQs778ajze1RxcmP6S8RVVc=
Expand Down
5 changes: 3 additions & 2 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/jetstack/cert-manager/pkg/acme/webhook/apis/acme/v1alpha1"
"github.com/jetstack/cert-manager/pkg/acme/webhook/cmd"
certmanagerv1 "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1"
)

var GroupName = os.Getenv("GROUP_NAME")
Expand Down Expand Up @@ -64,8 +65,8 @@ type customDNSProviderConfig struct {
// These fields will be set by users in the
// `issuer.spec.acme.dns01.providers.webhook.config` field.

//Email string `json:"email"`
//APIKeySecretRef v1alpha1.SecretKeySelector `json:"apiKeySecretRef"`
Email string `json:"email"`
APIKeySecretRef certmanagerv1.SecretKeySelector `json:"apiKeySecretRef"`
Copy link
Member

Choose a reason for hiding this comment

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

Not all of these fields are required by all DNS provider types, hence keeping the contents here generic. Can you revert these changes, and if you feel there could be additional clarification as to what a user should do here then add comments to explain it?

Copy link
Author

Choose a reason for hiding this comment

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

I don't think that I included them or maybe this is because they were defined within one of the example of the cert manager project. So, do you suggest that we remove the fields Email and APIKeySecretRef ?

}

// Name is used as the name for this DNS solver when referencing it on the ACME
Expand Down
33 changes: 31 additions & 2 deletions main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,54 @@
package main

import (
cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1"
logf "github.com/jetstack/cert-manager/pkg/logs"
"github.com/jetstack/cert-manager/test/acme/dns"
testserver "github.com/jetstack/cert-manager/test/acme/dns/server"
"os"
"testing"

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

var (
zone = os.Getenv("TEST_ZONE_NAME")
kubeBuilderBinPath = "./_out/kubebuilder/bin"
rfc2136TestFqdn = "_acme-challenge.123456789.www.example.com."
rfc2136TestZone = "example.com."
rfc2136TestTsigKeyName = "example.com."
rfc2136TestTsigSecret = "IwBTJx9wrDp4Y1RyC3H0gA=="
)

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.

ctx := logf.NewContext(nil, nil, t.Name())
server := &testserver.BasicServer{
Zones: []string{rfc2136TestZone},
EnableTSIG: true,
TSIGZone: rfc2136TestZone,
TSIGKeyName: rfc2136TestTsigKeyName,
TSIGKeySecret: rfc2136TestTsigSecret,
}
if err := server.Run(ctx); err != nil {
t.Fatalf("failed to start test server: %v", err)
}
defer server.Shutdown()

var validConfig = cmapi.ACMEIssuerDNS01ProviderRFC2136{
Nameserver: server.ListenAddr(),
}
Copy link
Member

Choose a reason for hiding this comment

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

Referencing the RFC2136 solver here is confusing to users, and also won't result in the RFC2136 issuer actually being used (as dns.NewFixture(&customDNSProviderSolver{} is still called).

Having the binaries path set + the script to fetch those binaries is great, but I think this example should be generic and show a basic setup that needs a bit of modification or commenting/uncommenting of example lines to get things to work.

Copy link
Author

Choose a reason for hiding this comment

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

So, what do you suggest that we do then here ?


fixture := dns.NewFixture(&customDNSProviderSolver{},
dns.SetBinariesPath(kubeBuilderBinPath),
dns.SetResolvedZone(zone),
dns.SetResolvedFQDN(rfc2136TestFqdn),
dns.SetConfig(validConfig),
dns.SetDNSServer(server.ListenAddr()),
dns.SetAllowAmbientCredentials(false),
dns.SetManifestPath("testdata/my-custom-solver"),
dns.SetUseAuthoritative(false),
)

fixture.RunConformance(t)
Expand Down
60 changes: 60 additions & 0 deletions scripts/fetch-test-binaries.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,61 @@
#!/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