Merge pull request #217 from munnerz/automated-cherry-pick-of-#214-#215…

…-upstream-release-0.2

Automatic merge from submit-queue.

Automated cherry pick of #214 #215

Cherry pick of #214 #215 on release-0.2.

#214: Update for v0.2.2
#215: Fix checking for expired ACME authorizations

contrib/charts/cert-manager/README.md

@@ -47,7 +47,7 @@ The following tables lists the configurable parameters of the cert-manager chart
 | Parameter              | Description                             | Default                                        |
 | ---------------------- | --------------------------------------- | ---------------------------------------------- |
 | `image.repository`     | Image repository                        | `quay.io/jetstack/cert-manager-controller`     |
-| `image.tag`            | Image tag                               | `v0.2.1`                                       |
+| `image.tag`            | Image tag                               | `v0.2.2`                                       |
 | `image.pullPolicy`     | Image pull policy                       | `Always`                                       |
 | `replicaCount`         | Number of cert-manager replicas         | `1`                                            |
 | `createCustomResource` | Create CRD/TPR with this release        | `true`                                         |

contrib/charts/cert-manager/values.yaml

@@ -5,7 +5,7 @@ replicaCount: 1
 
 image:
   repository: quay.io/jetstack/cert-manager-controller
-  tag: v0.2.1
+  tag: v0.2.2
   pullPolicy: Always
 
 createCustomResource: true
@@ -30,5 +30,5 @@ ingressShim:
     repository: quay.io/jetstack/cert-manager-ingress-shim
     # Defaults to image.tag.
     # You should only change this if you know what you are doing!
-    # tag: v0.2.1
+    # tag: v0.2.2
     pullPolicy: Always

docs/examples/cert-manager.yaml

@@ -13,5 +13,8 @@ spec:
     spec:
       containers:
       - name: cert-manager
-        image: quay.io/jetstack/cert-manager-controller:v0.2.1
+        image: quay.io/jetstack/cert-manager-controller:v0.2.2
+        imagePullPolicy: Always
+      - name: ingress-shim
+        image: quay.io/jetstack/cert-manager-ingress-shim:v0.2.2
         imagePullPolicy: Always

pkg/issuer/acme/prepare.go

@@ -195,6 +195,12 @@ func checkAuthorization(ctx context.Context, cl *acme.Client, uri string) (bool,
 	a, err := cl.GetAuthorization(ctx, uri)
 
 	if err != nil {
+		if err, ok := err.(*acme.Error); ok {
+			// response code is 404 when authorization has expired
+			if err.StatusCode == 404 {
+				return false, nil
+			}
+		}
 		return false, err
 	}
 

pkg/issuer/acme/prepare_test.go

@@ -1 +1,100 @@
 package acme
+
+import (
+	"context"
+	"io/ioutil"
+	"net/http"
+	"strings"
+	"testing"
+
+	"golang.org/x/crypto/acme"
+	"gopkg.in/jarcoal/httpmock.v1"
+)
+
+func TestCheckAuthorization(t *testing.T) {
+	type testT struct {
+		name           string
+		mockStatusCode int
+		mockResponse   string
+		ctx            context.Context
+		uri            string
+		expected       bool
+		err            bool
+	}
+	tests := []testT{
+		{
+			name:           "should return no error for 404 return code",
+			mockStatusCode: 404,
+			mockResponse: `{
+  "type": "urn:acme:error:malformed",
+  "detail": "Expired authorization",
+  "status": 404
+}`,
+			uri:      "http://testuri",
+			expected: false,
+			err:      false,
+		},
+		{
+			name:           "should return valid for a 200 return code",
+			mockStatusCode: 200,
+			mockResponse: `{
+  "status": "valid"
+}`,
+			uri:      "http://testuri",
+			expected: true,
+			err:      false,
+		},
+		{
+			name:           "should return invalid but no error for any status that isn't 'valid'",
+			mockStatusCode: 200,
+			mockResponse: `{
+  "status": "invalid"
+}`,
+			uri:      "http://testuri",
+			expected: false,
+			err:      false,
+		},
+		{
+			name:           "should return an error for an invalid response",
+			mockStatusCode: 500,
+			// invalid response body
+			mockResponse: `{
+  "type": "urn:acme:error:malformed",
+  "detail": "Fake error",
+  "status": 500
+}`,
+			uri:      "http://testuri",
+			expected: false,
+			err:      true,
+		},
+	}
+	testFn := func(test testT) func(t *testing.T) {
+		return func(t *testing.T) {
+			mock := httpmock.NewMockTransport()
+			mock.RegisterResponder("GET", test.uri, httpmock.ResponderFromResponse(&http.Response{
+				StatusCode: test.mockStatusCode,
+				Body:       ioutil.NopCloser(strings.NewReader(test.mockResponse)),
+			}))
+			ctx := test.ctx
+			if ctx == nil {
+				ctx = context.Background()
+			}
+			cl := &acme.Client{
+				HTTPClient: &http.Client{Transport: mock},
+			}
+			valid, err := checkAuthorization(ctx, cl, test.uri)
+			if err != nil && !test.err {
+				t.Errorf("expected no error, but got: %s", err)
+			}
+			if err == nil && test.err {
+				t.Errorf("expected error, but got no error")
+			}
+			if valid != test.expected {
+				t.Errorf("expected checkAuthorization to return %v, but got %v", test.expected, valid)
+			}
+		}
+	}
+	for _, test := range tests {
+		t.Run(test.name, testFn(test))
+	}
+}