Merge remote-tracking branch 'origin' into docs-CE556

.changelog/21361.txt

@@ -0,0 +1,8 @@
+```release-note:bug
+dns: Fix a regression where DNS tags using the standard lookup syntax, `tag.name.service.consul`, were being disregarded.
+```
+
+```release-note:bug
+dns: Fix a regression where DNS SRV questions were returning duplicate hostnames instead of encoded IPs.
+This affected Nomad integrations with Consul.
+```

.github/workflows/build.yml

@@ -95,6 +95,8 @@ jobs:
           - {goos: "linux", goarch: "amd64"}
           - {goos: "linux", goarch: "arm"}
           - {goos: "linux", goarch: "arm64"}
+          - {goos: "darwin", goarch: "amd64"}
+          - {goos: "darwin", goarch: "arm64"}
           - {goos: "freebsd", goarch: "386"}
           - {goos: "freebsd", goarch: "amd64"}
           - {goos: "windows", goarch: "386"}
@@ -246,58 +248,6 @@ jobs:
             cp LICENSE $TARGET_DIR/LICENSE.txt
             go build -ldflags="$GOLDFLAGS" -o "$BIN_PATH" -trimpath -buildvcs=false
 
-  build-darwin:
-    needs:
-    - set-product-version
-    - get-go-version
-    runs-on: macos-latest
-    strategy:
-      matrix:
-        goos: [ darwin ]
-        goarch: [ "amd64", "arm64" ]
-      fail-fast: true
-
-    name: Go ${{ needs.get-go-version.outputs.go-version }} ${{ matrix.goos }} ${{ matrix.goarch }} build
-    steps:
-      - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4
-
-      - name: Setup with node and yarn
-        uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
-        with:
-          node-version: '18'
-          cache: 'yarn'
-          cache-dependency-path: 'ui/yarn.lock'
-
-      - name: Build UI
-        run: |
-          CONSUL_VERSION=${{ needs.set-product-version.outputs.product-version }}
-          CONSUL_BINARY_TYPE=${CONSUL_BINARY_TYPE}
-          CONSUL_COPYRIGHT_YEAR=$(git show -s --format=%cd --date=format:%Y HEAD)
-          echo "consul_version is ${CONSUL_VERSION}"
-          echo "consul binary type is ${CONSUL_BINARY_TYPE}"
-          echo "consul copyright year is ${CONSUL_COPYRIGHT_YEAR}"
-          cd ui && make && cd ..
-          rm -rf agent/uiserver/dist
-          mv ui/packages/consul-ui/dist agent/uiserver/
-      - name: Go Build
-        env:
-          PRODUCT_VERSION: ${{ needs.set-product-version.outputs.product-version }}
-          PRERELEASE_VERSION: ${{ needs.set-product-version.outputs.pre-version }}
-          CGO_ENABLED: "0"
-          GOLDFLAGS: "${{needs.set-product-version.outputs.shared-ldflags}}"
-        uses: hashicorp/actions-go-build@make-clean-flag-optional
-        with:
-          product_name: ${{ env.PKG_NAME }}
-          product_version: ${{ needs.set-product-version.outputs.product-version }}
-          go_version: ${{ needs.get-go-version.outputs.go-version }}
-          os: ${{ matrix.goos }}
-          arch: ${{ matrix.goarch }}
-          reproducible: nope
-          clean: false
-          instructions: |-
-            cp LICENSE $TARGET_DIR/LICENSE.txt
-            go build -ldflags="$GOLDFLAGS" -tags netcgo -o "$BIN_PATH" -trimpath -buildvcs=false
-
   build-docker:
     name: Docker ${{ matrix.arch }} build
     needs:
@@ -420,32 +370,6 @@ jobs:
         if: ${{ endsWith(github.repository, '-enterprise') || matrix.arch != 's390x' }}
         run: .github/scripts/verify_artifact.sh ${{ env.zip_name }} v${{ env.version }}
 
-  verify-darwin:
-    needs:
-      - set-product-version
-      - build-darwin
-    runs-on: macos-latest
-    strategy:
-      fail-fast: true
-    env:
-      version: ${{needs.set-product-version.outputs.product-version}}
-      zip_name: consul_${{ needs.set-product-version.outputs.product-version }}_darwin_amd64.zip
-
-    name: Verify amd64 darwin binary
-    steps:
-      - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4
-
-      - name: Download amd64 darwin zip
-        uses: actions/download-artifact@65a9edc5881444af0b9093a5e628f2fe47ea3b2e # v4.1.7
-        with:
-          name: ${{ env.zip_name }}
-
-      - name: Unzip amd64 darwin zip
-        run: unzip ${{ env.zip_name }}
-
-      - name: Run verification for amd64 darwin binary
-        run: .github/scripts/verify_bin.sh ./consul v${{ env.version }}
-
   verify-linux-packages-deb:
     needs:
       - build

agent/dns/discovery_results_fetcher.go

@@ -106,7 +106,10 @@ func buildQueryFromDNSMessage(req *dns.Msg, reqCtx Context, domain, altDomain st
 		return nil, err
 	}
 
-	name, tag := getQueryNameAndTagFromParts(queryType, queryParts)
+	name, tag, err := getQueryNameAndTagFromParts(queryType, queryParts)
+	if err != nil {
+		return nil, err
+	}
 
 	portName := parsePort(queryParts)
 
@@ -157,14 +160,28 @@ func buildAddressResults(req *dns.Msg) ([]*discovery.Result, error) {
 }
 
 // getQueryNameAndTagFromParts returns the query name and tag from the query parts that are taken from the original dns question.
-func getQueryNameAndTagFromParts(queryType discovery.QueryType, queryParts []string) (string, string) {
+//
+// Valid Query Parts:
+// [<tag>.]<service>
+// [<port>.port.]<service>
+// _<service>._<tag> // RFC 2782 style
+func getQueryNameAndTagFromParts(queryType discovery.QueryType, queryParts []string) (string, string, error) {
 	n := len(queryParts)
 	if n == 0 {
-		return "", ""
+		return "", "", errInvalidQuestion
 	}
 
 	switch queryType {
 	case discovery.QueryTypeService:
+		if n > 3 {
+			// Having this many fields is never valid.
+			return "", "", errInvalidQuestion
+		}
+		if n == 3 && queryParts[n-2] != "port" {
+			// This probably means that someone was trying to use a tag name with a period.
+			// This was deprecated in Consul 0.3.
+			return "", "", errInvalidQuestion
+		}
 		// Support RFC 2782 style syntax
 		if n == 2 && strings.HasPrefix(queryParts[1], "_") && strings.HasPrefix(queryParts[0], "_") {
 			// Grab the tag since we make nuke it if it's tcp
@@ -177,9 +194,14 @@ func getQueryNameAndTagFromParts(queryType discovery.QueryType, queryParts []str
 
 			name := queryParts[0][1:]
 			// _name._tag.service.consul
-			return name, tag
+			return name, tag, nil
 		}
-		return queryParts[n-1], ""
+		// Standard-style lookup w/ tag
+		if n == 2 {
+			return queryParts[1], queryParts[0], nil
+		}
+		// This works for the v1 and v2 catalog queries, even if a port name was specified.
+		return queryParts[n-1], "", nil
 	case discovery.QueryTypePreparedQuery:
 		name := ""
 
@@ -197,9 +219,17 @@ func getQueryNameAndTagFromParts(queryType discovery.QueryType, queryParts []str
 			// Allow a "." in the query name, just join all the parts.
 			name = strings.Join(queryParts, ".")
 		}
-		return name, ""
+
+		if name == "" {
+			return "", "", errInvalidQuestion
+		}
+		return name, "", nil
+	}
+	name := queryParts[n-1]
+	if name == "" {
+		return "", "", errInvalidQuestion
 	}
-	return queryParts[n-1], ""
+	return queryParts[n-1], "", nil
 }
 
 // getQueryTenancy returns a discovery.QueryTenancy from a DNS message.

agent/dns/discovery_results_fetcher_test.go

@@ -19,6 +19,7 @@ type testCaseBuildQueryFromDNSMessage struct {
 	request        *dns.Msg
 	requestContext *Context
 	expectedQuery  *discovery.Query
+	expectedError  string
 }
 
 // Test_buildQueryFromDNSMessage tests the buildQueryFromDNSMessage function.
@@ -123,6 +124,114 @@ func Test_buildQueryFromDNSMessage(t *testing.T) {
 				},
 			},
 		},
+		// V1 Service Queries
+		{
+			name: "test A 'service.' standard query with tag",
+			request: &dns.Msg{
+				MsgHdr: dns.MsgHdr{
+					Opcode: dns.OpcodeQuery,
+				},
+				Question: []dns.Question{
+					{
+						Name:   "primary.db.service.dc1.consul", // "intentionally missing the trailing dot"
+						Qtype:  dns.TypeA,
+						Qclass: dns.ClassINET,
+					},
+				},
+			},
+			expectedQuery: &discovery.Query{
+				QueryType: discovery.QueryTypeService,
+				QueryPayload: discovery.QueryPayload{
+					Name: "db",
+					Tag:  "primary",
+					Tenancy: discovery.QueryTenancy{
+						Datacenter: "dc1",
+					},
+				},
+			},
+		},
+		{
+			name: "test A 'service.' RFC 2782 query with tag",
+			request: &dns.Msg{
+				MsgHdr: dns.MsgHdr{
+					Opcode: dns.OpcodeQuery,
+				},
+				Question: []dns.Question{
+					{
+						Name:   "_db._primary.service.dc1.consul", // "intentionally missing the trailing dot"
+						Qtype:  dns.TypeA,
+						Qclass: dns.ClassINET,
+					},
+				},
+			},
+			expectedQuery: &discovery.Query{
+				QueryType: discovery.QueryTypeService,
+				QueryPayload: discovery.QueryPayload{
+					Name: "db",
+					Tag:  "primary",
+					Tenancy: discovery.QueryTenancy{
+						Datacenter: "dc1",
+					},
+				},
+			},
+		},
+		{
+			name: "test A 'service.' RFC 2782 query",
+			request: &dns.Msg{
+				MsgHdr: dns.MsgHdr{
+					Opcode: dns.OpcodeQuery,
+				},
+				Question: []dns.Question{
+					{
+						Name:   "_db._tcp.service.dc1.consul", // the `tcp` tag should be ignored
+						Qtype:  dns.TypeA,
+						Qclass: dns.ClassINET,
+					},
+				},
+			},
+			expectedQuery: &discovery.Query{
+				QueryType: discovery.QueryTypeService,
+				QueryPayload: discovery.QueryPayload{
+					Name: "db",
+					Tenancy: discovery.QueryTenancy{
+						Datacenter: "dc1",
+					},
+				},
+			},
+		},
+		{
+			name: "test A 'service.' with too many query parts (RFC 2782 style)",
+			request: &dns.Msg{
+				MsgHdr: dns.MsgHdr{
+					Opcode: dns.OpcodeQuery,
+				},
+				Question: []dns.Question{
+					{
+						Name:   "nope._db._tcp.service.dc1.consul", // the `tcp` tag should be ignored
+						Qtype:  dns.TypeA,
+						Qclass: dns.ClassINET,
+					},
+				},
+			},
+			expectedError: "invalid question",
+		},
+		{
+			name: "test A 'service.' with too many query parts (standard style)",
+			request: &dns.Msg{
+				MsgHdr: dns.MsgHdr{
+					Opcode: dns.OpcodeQuery,
+				},
+				Question: []dns.Question{
+					{
+						Name:   "too.many.parts.service.dc1.consul.",
+						Qtype:  dns.TypeA,
+						Qclass: dns.ClassINET,
+					},
+				},
+			},
+			expectedError: "invalid question",
+		},
+		// V2 Catalog Queries
 		{
 			name: "test A 'workload.'",
 			request: &dns.Msg{
@@ -213,6 +322,13 @@ func Test_buildQueryFromDNSMessage(t *testing.T) {
 				context = &Context{}
 			}
 			query, err := buildQueryFromDNSMessage(tc.request, *context, "consul.", ".", nil)
+
+			if tc.expectedError != "" {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tc.expectedError)
+				return
+			}
+
 			require.NoError(t, err)
 			assert.Equal(t, tc.expectedQuery, query)
 		})