Skip to content

Commit db6bf4c

Browse files
edwardrfedw-defanglionello
authored
Consider redirection a success response for cert gen trigger (#832)
* Consider redirection a success response for cert gen trigger * Consider redirect a success cert gen trigger * Apply suggestions from code review Co-authored-by: Lio李歐 <[email protected]> --------- Co-authored-by: Edward J <[email protected]> Co-authored-by: Lio李歐 <[email protected]>
1 parent c656181 commit db6bf4c

File tree

2 files changed

+88
-38
lines changed

2 files changed

+88
-38
lines changed

‎src/pkg/cli/cert.go

Lines changed: 50 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package cli
22

33
import (
44
"context"
5+
"crypto/tls"
56
"errors"
67
"fmt"
78
"io"
@@ -111,7 +112,7 @@ func GenerateLetsEncryptCert(ctx context.Context, client client.FabricClient, pr
111112
}
112113

113114
func generateCert(ctx context.Context, domain string, targets []string, client client.FabricClient) {
114-
term.Infof("Triggering TLS cert generation for %v", domain)
115+
term.Infof("Checking DNS setup for %v", domain)
115116
if err := waitForCNAME(ctx, domain, targets, client); err != nil {
116117
term.Errorf("Error waiting for CNAME: %v", err)
117118
return
@@ -198,7 +199,6 @@ func waitForCNAME(ctx context.Context, domain string, targets []string, client c
198199
ticker := time.NewTicker(5 * time.Second)
199200
defer ticker.Stop()
200201

201-
msgShown := false
202202
serverSideVerified := false
203203
serverVerifyRpcFailure := 0
204204
doSpinner := term.StdoutCanColor() && term.IsTerminal()
@@ -211,41 +211,50 @@ func waitForCNAME(ctx context.Context, domain string, targets []string, client c
211211
defer cancelSpinner()
212212
}
213213

214+
verifyDNS := func() error {
215+
if !serverSideVerified && serverVerifyRpcFailure < 3 {
216+
if err := client.VerifyDNSSetup(ctx, &defangv1.VerifyDNSSetupRequest{Domain: domain, Targets: targets}); err == nil {
217+
term.Debugf("Server side DNS verification for %v successful", domain)
218+
serverSideVerified = true
219+
} else {
220+
if cerr := new(connect.Error); errors.As(err, &cerr) && cerr.Code() == connect.CodeFailedPrecondition {
221+
term.Debugf("Server side DNS verification negative result: %v", cerr.Message())
222+
} else {
223+
term.Debugf("Server side DNS verification request for %v failed: %v", domain, err)
224+
serverVerifyRpcFailure++
225+
}
226+
}
227+
if serverVerifyRpcFailure >= 3 {
228+
term.Warnf("Server side DNS verification for %v failed multiple times, skipping server side DNS verification.", domain)
229+
}
230+
}
231+
if serverSideVerified || serverVerifyRpcFailure >= 3 {
232+
locallyVerified := dns.CheckDomainDNSReady(ctx, domain, targets)
233+
if serverSideVerified && !locallyVerified {
234+
term.Warnf("The DNS configuration for %v has been successfully verified. However, your local environment may still be using cached data, so it could take several minutes for the DNS changes to propagate on your system.", domain)
235+
return nil
236+
}
237+
if locallyVerified {
238+
return nil
239+
}
240+
}
241+
return errors.New("not verified")
242+
}
243+
244+
if err := verifyDNS(); err == nil {
245+
return nil
246+
}
247+
term.Infof("Configure a CNAME or ALIAS record for the domain name: %v", domain)
248+
fmt.Printf(" %v -> %v\n", domain, strings.Join(targets, " or "))
249+
term.Infof("Awaiting DNS record setup and propagation... This may take a while.")
250+
214251
for {
215252
select {
216253
case <-ctx.Done():
217254
return ctx.Err()
218255
case <-ticker.C:
219-
if !serverSideVerified && serverVerifyRpcFailure < 3 {
220-
if err := client.VerifyDNSSetup(ctx, &defangv1.VerifyDNSSetupRequest{Domain: domain, Targets: targets}); err == nil {
221-
term.Debugf("Server side DNS verification for %v successful", domain)
222-
serverSideVerified = true
223-
} else {
224-
if cerr := new(connect.Error); errors.As(err, &cerr) && cerr.Code() == connect.CodeFailedPrecondition {
225-
term.Debugf("Server side DNS verification negative result: %v", cerr.Message())
226-
} else {
227-
term.Debugf("Server side DNS verification request for %v failed: %v", domain, err)
228-
serverVerifyRpcFailure++
229-
}
230-
}
231-
if serverVerifyRpcFailure >= 3 {
232-
term.Warnf("Server side DNS verification for %v failed multiple times, skipping server side DNS verification.", domain)
233-
}
234-
} else {
235-
locallyVerified := dns.CheckDomainDNSReady(ctx, domain, targets)
236-
if serverSideVerified && !locallyVerified {
237-
term.Warnf("The DNS configuration for %v has been successfully verified. However, your local environment may still be using cached data, so it could take several minutes for the DNS changes to propagate on your system.", domain)
238-
return nil
239-
}
240-
if locallyVerified {
241-
return nil
242-
}
243-
}
244-
if !msgShown {
245-
term.Infof("Please set up a CNAME record for %v", domain)
246-
fmt.Printf(" %v CNAME or as an alias to [ %v ]\n", domain, strings.Join(targets, " or "))
247-
term.Infof("Waiting for CNAME record setup and DNS propagation...")
248-
msgShown = true
256+
if err := verifyDNS(); err == nil {
257+
return nil
249258
}
250259
}
251260
}
@@ -261,14 +270,20 @@ func getWithRetries(ctx context.Context, url string, tries int) error {
261270
resp, err := httpClient.Do(req)
262271
if err == nil {
263272
defer resp.Body.Close()
264-
var msg []byte
265-
msg, err = io.ReadAll(resp.Body)
273+
_, err = io.ReadAll(resp.Body) // Read the body to ensure the request is not swallowed by alb
266274
if resp.StatusCode == http.StatusOK {
267275
return nil
268276
}
277+
if resp != nil && resp.Request != nil && resp.Request.URL.Scheme == "https" {
278+
term.Debugf("cert gen request success, received redirect to %v", resp.Request.URL)
279+
return nil // redirect to https indicate a successful cert generation
280+
}
269281
if err == nil {
270-
err = fmt.Errorf("HTTP %v: %v", resp.StatusCode, string(msg))
282+
err = fmt.Errorf("HTTP: %v", resp.StatusCode)
271283
}
284+
} else if cve := new(tls.CertificateVerificationError); errors.As(err, &cve) {
285+
term.Debugf("cert gen request success, received tls error: %v", cve)
286+
return nil // tls error indicate a successful cert gen trigger, as it has to be redirected to https
272287
}
273288

274289
term.Debugf("Error fetching %v: %v, tries left %v", url, err, tries-i-1)

‎src/pkg/cli/cert_test.go

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package cli
22

33
import (
44
"context"
5+
"crypto/tls"
56
"errors"
67
"fmt"
78
"io"
@@ -32,6 +33,9 @@ func (c *testClient) Do(req *http.Request) (*http.Response, error) {
3233
}
3334
tr := c.tries[0]
3435
c.tries = c.tries[1:]
36+
if tr.result != nil && tr.result.Request == nil {
37+
tr.result.Request = req
38+
}
3539
return tr.result, tr.err
3640
}
3741

@@ -88,13 +92,44 @@ func TestGetWithRetries(t *testing.T) {
8892
err := getWithRetries(context.Background(), "http://example.com", 3)
8993
if err == nil {
9094
t.Errorf("Expected error, got %v", err)
91-
} else if !strings.Contains(err.Error(), "HTTP 503: Random Error") {
92-
t.Errorf("Expected HTTP 503: Random Error, got %v", err)
95+
} else if !strings.Contains(err.Error(), "HTTP: 503") {
96+
t.Errorf("Expected HTTP 503:, got %v", err)
9397
}
9498
if tc.calls != 3 {
9599
t.Errorf("Expected 3 calls, got %v", tc.calls)
96100
}
97101
})
102+
t.Run("redirect to https considers success", func(t *testing.T) {
103+
redirectURL, _ := url.Parse("https://example.com")
104+
tc := &testClient{tries: []tryResult{
105+
{result: &http.Response{StatusCode: 503, Request: &http.Request{URL: redirectURL}, Body: mockBody("Random Error")}, err: nil},
106+
}}
107+
originalClient := httpClient
108+
t.Cleanup(func() { httpClient = originalClient })
109+
httpClient = tc
110+
err := getWithRetries(context.Background(), "http://example.com", 3)
111+
if err != nil {
112+
t.Errorf("Expected no error, got %v", err)
113+
}
114+
if tc.calls != 1 {
115+
t.Errorf("Expected 1 call, got %v", tc.calls)
116+
}
117+
})
118+
t.Run("TLS error considers success", func(t *testing.T) {
119+
tc := &testClient{tries: []tryResult{
120+
{result: nil, err: &tls.CertificateVerificationError{Err: errors.New("error")}},
121+
}}
122+
originalClient := httpClient
123+
t.Cleanup(func() { httpClient = originalClient })
124+
httpClient = tc
125+
err := getWithRetries(context.Background(), "http://example.com", 3)
126+
if err != nil {
127+
t.Errorf("Expected no error, got %v", err)
128+
}
129+
if tc.calls != 1 {
130+
t.Errorf("Expected 1 call, got %v", tc.calls)
131+
}
132+
})
98133
t.Run("handles all http errors", func(t *testing.T) {
99134
tc := &testClient{tries: []tryResult{
100135
{result: &http.Response{StatusCode: 404, Body: mockBody("Random Error")}, err: nil},
@@ -107,7 +142,7 @@ func TestGetWithRetries(t *testing.T) {
107142
err := getWithRetries(context.Background(), "http://example.com", 3)
108143
if err == nil {
109144
t.Errorf("Expected error, got %v", err)
110-
} else if !strings.Contains(err.Error(), "HTTP 404: Random Error") || !strings.Contains(err.Error(), "HTTP 502: Random Error") || !strings.Contains(err.Error(), "HTTP 503: Random Error") {
145+
} else if !strings.Contains(err.Error(), "HTTP: 404") || !strings.Contains(err.Error(), "HTTP: 502") || !strings.Contains(err.Error(), "HTTP: 503") {
111146
t.Errorf("Expected HTTP 404,502,503 erros, got %v", err)
112147
}
113148
if tc.calls != 3 {

0 commit comments

Comments
 (0)