Merge branch 'main' into mattm-refactor-ticker

Author: mcpherrinm

.github/workflows/release.yml

@@ -1,21 +1,42 @@
 # Build the Boulder Debian package on tag push, and attach it to a GitHub
 # release.
 #
-# Keep in sync with try-release.yml, with the exception that try-release.yml can
-# have multiple entries in its matrix but this should only have one.
+# Keep the GO_VERSION matrix and the container-building steps in sync with
+# try-release.yml.
 name: Build release
 on:
   push:
     tags:
       - '**'
 
 jobs:
+  draft-release:
+    runs-on: ubuntu-24.04
+    permissions:
+      contents: write
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          persist-credentials: false
+          fetch-depth: '0' # Needed for verify-release-ancestry.sh to see origin/main
+
+      - name: Verify release ancestry
+        run: ./tools/verify-release-ancestry.sh "$GITHUB_SHA"
+
+      - name: Create draft release
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        # https://cli.github.com/manual/gh_release_create
+        run: gh release create --draft --generate-notes "${GITHUB_REF_NAME}"
+
   push-release:
+    needs: draft-release
     strategy:
       fail-fast: false
       matrix:
         GO_VERSION:
           - "1.25.2"
+          - "1.25.3"
     runs-on: ubuntu-24.04
     permissions:
       contents: write
@@ -26,47 +47,55 @@ jobs:
           persist-credentials: false
           fetch-depth: '0' # Needed for verify-release-ancestry.sh to see origin/main
 
-      - name: Verify release ancestry
-        run: ./tools/verify-release-ancestry.sh "$GITHUB_SHA"
-
       - name: Build Boulder container and .deb
         id: build
         env:
           GO_VERSION: ${{ matrix.GO_VERSION }}
         run: ./tools/container-build.sh
 
       - name: Tag Boulder container
-        run: docker tag boulder "ghcr.io/letsencrypt/boulder:${{ github.ref_name }}-go${{ matrix.GO_VERSION }}"
+        run: docker tag boulder "ghcr.io/letsencrypt/boulder:${GITHUB_REF_NAME}-go${{ matrix.GO_VERSION }}"
 
       - name: Compute checksums
         id: checksums
         # The files listed on this line must be identical to the files uploaded
         # in the last step.
         run: sha256sum boulder*.deb boulder*.tar.gz >| boulder-${{ matrix.GO_VERSION }}.$(date +%s)-$(git rev-parse --short=8 HEAD).checksums.txt
 
-      - name: Create release
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-        # https://cli.github.com/manual/gh_release_create
-        run: gh release create --generate-notes "${GITHUB_REF_NAME}"
-        continue-on-error: true
-
       - name: Upload release files
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         # https://cli.github.com/manual/gh_release_upload
         run: gh release upload "${GITHUB_REF_NAME}" boulder*.deb boulder*.tar.gz boulder*.checksums.txt
 
       - name: Build ct-test-srv container
-        run: docker buildx build . --build-arg "GO_VERSION=${{ matrix.GO_VERSION }}" -f test/ct-test-srv/Dockerfile -t "ghcr.io/letsencrypt/ct-test-srv:${{ github.ref_name }}-go${{ matrix.GO_VERSION }}"
+        run: docker buildx build . --build-arg "GO_VERSION=${{ matrix.GO_VERSION }}" -f test/ct-test-srv/Dockerfile -t "ghcr.io/letsencrypt/ct-test-srv:${GITHUB_REF_NAME}-go${{ matrix.GO_VERSION }}"
 
-      - name: Login to ghcr.io
-        run: printenv GITHUB_TOKEN | docker login ghcr.io -u "${{ github.actor }}" --password-stdin
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      - name: Login to GitHub Container Registry
+        uses: docker/login-action@343f7c4344506bcbf9b4de18042ae17996df046d # v3.0.0
+        with:
+          registry: ghcr.io
+          username: ${{ github.actor }}
+          password: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Push Boulder container
-        run: docker push "ghcr.io/letsencrypt/boulder:${{ github.ref_name }}-go${{ matrix.GO_VERSION }}"
+        run: docker push "ghcr.io/letsencrypt/boulder:${GITHUB_REF_NAME}-go${{ matrix.GO_VERSION }}"
 
       - name: Push ct-test-srv container
-        run: docker push "ghcr.io/letsencrypt/ct-test-srv:${{ github.ref_name }}-go${{ matrix.GO_VERSION }}"
+        run: docker push "ghcr.io/letsencrypt/ct-test-srv:${GITHUB_REF_NAME}-go${{ matrix.GO_VERSION }}"
+
+  publish-release:
+    needs: push-release
+    runs-on: ubuntu-24.04
+    permissions:
+      contents: write
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          persist-credentials: false
+
+      - name: Publish release
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        # https://cli.github.com/manual/gh_release_edit
+        run: gh release edit --draft=false "${GITHUB_REF_NAME}"

.github/workflows/try-release.yml

@@ -1,7 +1,8 @@
-# Try building the Boulder Debian package on every PR and push to main.
-# This is to make sure the actual release job will succeed when we tag a
-# release.
-# Keep in sync with release.yml
+# Try building the Boulder Debian package on every PR and push to main. This is
+# to make sure the actual release job will succeed when we tag a release.
+#
+# Keep the GO_VERSION matrix and the container-building steps in sync with
+# release.yml.
 name: Try release
 on:
   push:
@@ -20,6 +21,7 @@ jobs:
       matrix:
         GO_VERSION:
           - "1.25.2"
+          - "1.25.3"
     runs-on: ubuntu-24.04
     steps:
       - uses: actions/checkout@v4

README.md

@@ -112,43 +112,44 @@ docker compose up
 To run our standard battery of tests (lints, unit, integration):
 
 ```shell
-docker compose run --use-aliases boulder ./test.sh
+./t.sh
 ```
 
 To run all unit tests:
 
 ```shell
-docker compose run --use-aliases boulder ./test.sh --unit
+./t.sh -u
 ```
 
 To run specific unit tests (example is of the ./va directory):
 
 ```shell
-docker compose run --use-aliases boulder ./test.sh --unit --filter=./va
+./t.sh -u -p ./va
 ```
 
 To run all integration tests:
 
 ```shell
-docker compose run --use-aliases boulder ./test.sh --integration
+./t.sh -i
 ```
 
 To run unit tests and integration tests with coverage:
 
 ```shell
-docker compose run --use-aliases boulder ./test.sh --unit --integration --coverage --coverage-dir=./test/coverage/mytestrun
+./t.sh -ui -c --coverage-dir=./test/coverage/mytestrun
 ```
 
 To run specific integration tests (example runs TestGenerateValidity and TestWFECORS):
 
 ```shell
-docker compose run --use-aliases boulder ./test.sh --filter TestGenerateValidity/TestWFECORS
+./t.sh -i -f TestGenerateValidity/TestWFECORS
 ```
 
-To get a list of available integration tests:
+To do any of the above, but using the "config-next" configuration, which
+represents a likely future state (e.g. including new feature flags):
 
 ```shell
-docker compose run --use-aliases boulder ./test.sh --list-integration-tests
+./tn.sh -your -options -here
 ```
 
 The configuration in docker-compose.yml mounts your boulder checkout at
@@ -186,7 +187,13 @@ docker compose run --use-aliases -e FAKE_DNS=172.17.0.1 --service-ports boulder
 
 Running tests without the `./test.sh` wrapper:
 
-Run all unit tests
+Run unit tests locally, without docker (only works for some directories):
+
+```shell
+go test ./issuance/...
+```
+
+Run all unit tests:
 
 ```shell
 docker compose run --use-aliases boulder go test -p 1 ./...

bdns/dns.go

@@ -3,14 +3,12 @@ package bdns
 import (
 	"context"
 	"crypto/tls"
-	"encoding/base64"
 	"errors"
 	"fmt"
 	"io"
 	"net"
 	"net/http"
 	"net/netip"
-	"net/url"
 	"slices"
 	"strconv"
 	"strings"
@@ -48,10 +46,9 @@ type impl struct {
 	clk                      clock.Clock
 	log                      blog.Logger
 
-	queryTime         *prometheus.HistogramVec
-	totalLookupTime   *prometheus.HistogramVec
-	timeoutCounter    *prometheus.CounterVec
-	idMismatchCounter *prometheus.CounterVec
+	queryTime       *prometheus.HistogramVec
+	totalLookupTime *prometheus.HistogramVec
+	timeoutCounter  *prometheus.CounterVec
 }
 
 var _ Client = &impl{}
@@ -118,14 +115,6 @@ func New(
 		},
 		[]string{"qtype", "type", "resolver", "isTLD"},
 	)
-	idMismatchCounter := prometheus.NewCounterVec(
-		prometheus.CounterOpts{
-			Name: "dns_id_mismatch",
-			Help: "Counter of DNS ErrId errors sliced by query type and resolver",
-		},
-		[]string{"qtype", "resolver"},
-	)
-	stats.MustRegister(queryTime, totalLookupTime, timeoutCounter, idMismatchCounter)
 	return &impl{
 		dnsClient:                client,
 		servers:                  servers,
@@ -135,7 +124,6 @@ func New(
 		queryTime:                queryTime,
 		totalLookupTime:          totalLookupTime,
 		timeoutCounter:           timeoutCounter,
-		idMismatchCounter:        idMismatchCounter,
 		log:                      log,
 	}
 }
@@ -230,13 +218,11 @@ func (dnsClient *impl) exchangeOne(ctx context.Context, hostname string, qtype u
 				result = dns.RcodeToString[rsp.Rcode]
 			}
 			if err != nil {
-				logDNSError(dnsClient.log, chosenServer, hostname, m, rsp, err)
-				if err == dns.ErrId {
-					dnsClient.idMismatchCounter.With(prometheus.Labels{
-						"qtype":    qtypeStr,
-						"resolver": chosenServerIP,
-					}).Inc()
-				}
+				dnsClient.log.Infof("logDNSError chosenServer=[%s] hostname=[%s] queryType=[%s] err=[%s]",
+					chosenServer,
+					hostname,
+					qtypeStr,
+					err)
 			}
 			dnsClient.queryTime.With(prometheus.Labels{
 				"qtype":    qtypeStr,
@@ -273,10 +259,10 @@ func (dnsClient *impl) exchangeOne(ctx context.Context, hostname string, qtype u
 		case r := <-ch:
 			if r.err != nil {
 				var isRetryable bool
-				// According to the http package documentation, retryable
-				// errors emitted by the http package are of type *url.Error.
-				var urlErr *url.Error
-				isRetryable = errors.As(r.err, &urlErr) && urlErr.Temporary()
+				// Check if the error is a timeout error. Network errors
+				// that can timeout implement the net.Error interface.
+				var netErr net.Error
+				isRetryable = errors.As(r.err, &netErr) && netErr.Timeout()
 				hasRetriesLeft := tries < dnsClient.maxTries
 				if isRetryable && hasRetriesLeft {
 					tries++
@@ -469,68 +455,6 @@ func (dnsClient *impl) LookupCAA(ctx context.Context, hostname string) ([]*dns.C
 	return CAAs, response, ResolverAddrs{resolver}, nil
 }
 
-// logDNSError logs the provided err result from making a query for hostname to
-// the chosenServer. If the err is a `dns.ErrId` instance then the Base64
-// encoded bytes of the query (and if not-nil, the response) in wire format
-// is logged as well. This function is called from exchangeOne only for the case
-// where an error occurs querying a hostname that indicates a problem between
-// the VA and the chosenServer.
-func logDNSError(
-	logger blog.Logger,
-	chosenServer string,
-	hostname string,
-	msg, resp *dns.Msg,
-	underlying error) {
-	// We don't expect logDNSError to be called with a nil msg or err but
-	// if it happens return early. We allow resp to be nil.
-	if msg == nil || len(msg.Question) == 0 || underlying == nil {
-		return
-	}
-	queryType := dns.TypeToString[msg.Question[0].Qtype]
-
-	// If the error indicates there was a query/response ID mismatch then we want
-	// to log more detail.
-	if underlying == dns.ErrId {
-		packedMsgBytes, err := msg.Pack()
-		if err != nil {
-			logger.Errf("logDNSError failed to pack msg: %v", err)
-			return
-		}
-		encodedMsg := base64.StdEncoding.EncodeToString(packedMsgBytes)
-
-		var encodedResp string
-		var respQname string
-		if resp != nil {
-			packedRespBytes, err := resp.Pack()
-			if err != nil {
-				logger.Errf("logDNSError failed to pack resp: %v", err)
-				return
-			}
-			encodedResp = base64.StdEncoding.EncodeToString(packedRespBytes)
-			if len(resp.Answer) > 0 && resp.Answer[0].Header() != nil {
-				respQname = resp.Answer[0].Header().Name
-			}
-		}
-
-		logger.Infof(
-			"logDNSError ID mismatch chosenServer=[%s] hostname=[%s] respHostname=[%s] queryType=[%s] msg=[%s] resp=[%s] err=[%s]",
-			chosenServer,
-			hostname,
-			respQname,
-			queryType,
-			encodedMsg,
-			encodedResp,
-			underlying)
-	} else {
-		// Otherwise log a general DNS error
-		logger.Infof("logDNSError chosenServer=[%s] hostname=[%s] queryType=[%s] err=[%s]",
-			chosenServer,
-			hostname,
-			queryType,
-			underlying)
-	}
-}
-
 type dohExchanger struct {
 	clk       clock.Clock
 	hc        http.Client