Merge pull request #158 from munnerz/automated-cherry-pick-of-#156-up…

…stream-release-0.1

Automatic merge from submit-queue.

Automated cherry pick of #156

Cherry pick of #156 on release-0.1.

#156: Ensure ACME HTTP01 reachability test passes 5 times before

pkg/issuer/acme/http/http.go

@@ -58,6 +58,9 @@ type Solver struct {
 	secretLister corev1listers.SecretLister
 	solverImage  string
 
+	testReachability reachabilityTest
+	requiredPasses   int
+
 	// This is a hack to record the randomly generated names of resources
 	// created by this Solver. This should be refactored out in future with a
 	// redesign of this package. It is used so resources can be cleaned up
@@ -69,16 +72,20 @@ type Solver struct {
 	lock sync.Mutex
 }
 
+type reachabilityTest func(ctx context.Context, domain, path, key string) error
+
 // NewSolver returns a new ACME HTTP01 solver for the given Issuer and client.
 func NewSolver(issuer v1alpha1.GenericIssuer, client kubernetes.Interface, secretLister corev1listers.SecretLister, solverImage string) *Solver {
 	return &Solver{
-		issuer:       issuer,
-		client:       client,
-		secretLister: secretLister,
-		solverImage:  solverImage,
-		svcNames:     make(map[string]string),
-		ingNames:     make(map[string]string),
-		podNames:     make(map[string]string),
+		issuer:           issuer,
+		client:           client,
+		secretLister:     secretLister,
+		solverImage:      solverImage,
+		testReachability: testReachability,
+		requiredPasses:   5,
+		svcNames:         make(map[string]string),
+		ingNames:         make(map[string]string),
+		podNames:         make(map[string]string),
 	}
 }
 
@@ -401,6 +408,7 @@ func (s *Solver) Present(ctx context.Context, crt *v1alpha1.Certificate, domain,
 // routes to include the HTTP01 challenge path, or return with an error if the
 // context deadline is exceeded.
 func (s *Solver) Wait(ctx context.Context, crt *v1alpha1.Certificate, domain, token, key string) error {
+	passes := 0
 	ctx, cancel := context.WithTimeout(ctx, HTTP01Timeout)
 	defer cancel()
 	for {
@@ -409,16 +417,22 @@ func (s *Solver) Wait(ctx context.Context, crt *v1alpha1.Certificate, domain, to
 			out := make(chan error, 1)
 			go func() {
 				defer close(out)
-				out <- testReachability(ctx, domain, fmt.Sprintf("%s/%s", solver.HTTPChallengePath, token), key)
+				out <- s.testReachability(ctx, domain, fmt.Sprintf("%s/%s", solver.HTTPChallengePath, token), key)
 			}()
 			return out
 		}():
 			if err != nil {
+				passes = 0
 				glog.V(4).Infof("ACME HTTP01 self check failed for domain %q, waiting 5s: %v", domain, err)
 				time.Sleep(time.Second * 5)
 				continue
 			}
-			glog.V(4).Infof("ACME HTTP01 self check for %q passed", domain)
+			passes++
+			glog.V(4).Infof("ACME HTTP01 self check for %q passed (%d/%d)", domain, passes, s.requiredPasses+1)
+			if passes < s.requiredPasses {
+				time.Sleep(time.Second * 2)
+				continue
+			}
 			return nil
 		case <-ctx.Done():
 			return ctx.Err()

pkg/issuer/acme/http/http_test.go

@@ -0,0 +1,82 @@
+package http
+
+import (
+	"context"
+	"fmt"
+	"testing"
+	"time"
+)
+
+// contextWithTimeout calls context.WithTimeout, and throws away the cancel fn
+func contextWithTimeout(t time.Duration) context.Context {
+	c, _ := context.WithTimeout(context.Background(), t)
+	return c
+}
+
+// countReachabilityTestCalls is a wrapper function that allows us to count the number
+// of calls to a reachabilityTest.
+func countReachabilityTestCalls(counter *int, t reachabilityTest) reachabilityTest {
+	return func(ctx context.Context, domain, path, key string) error {
+		*counter++
+		return t(ctx, domain, path, key)
+	}
+}
+
+func TestWait(t *testing.T) {
+	type testT struct {
+		name               string
+		reachabilityTest   func(ctx context.Context, domain, path, key string) error
+		ctx                context.Context
+		domain, token, key string
+		expectedErr        error
+	}
+	tests := []testT{
+		{
+			name: "should pass",
+			reachabilityTest: func(context.Context, string, string, string) error {
+				return nil
+			},
+			ctx: contextWithTimeout(time.Second * 30),
+		},
+		{
+			name: "should timeout",
+			reachabilityTest: func(context.Context, string, string, string) error {
+				return fmt.Errorf("failed")
+			},
+			expectedErr: fmt.Errorf("context deadline exceeded"),
+			ctx:         contextWithTimeout(time.Second * 30),
+		},
+	}
+
+	for i := range tests {
+		test := tests[i]
+		t.Run(test.name, func(t *testing.T) {
+			calls := 0
+			requiredCallsForPass := 5
+			s := Solver{
+				testReachability: countReachabilityTestCalls(&calls, test.reachabilityTest),
+				requiredPasses:   requiredCallsForPass,
+			}
+
+			err := s.Wait(test.ctx, nil, test.domain, test.token, test.key)
+			if err != nil && test.expectedErr == nil {
+				t.Errorf("Expected Wait to return non-nil error, but got %v", err)
+				return
+			}
+			if err != nil && test.expectedErr != nil {
+				if err.Error() != test.expectedErr.Error() {
+					t.Errorf("Expected error %v from Wait, but got: %v", test.expectedErr, err)
+					return
+				}
+			}
+			if err == nil && test.expectedErr != nil {
+				t.Errorf("Expected error %v from Wait, but got none", test.expectedErr)
+				return
+			}
+			if test.expectedErr == nil && calls != requiredCallsForPass {
+				t.Errorf("Expected Wait to verify reachability test passes %d times, but only checked %d", requiredCallsForPass, calls)
+				return
+			}
+		})
+	}
+}