Skip to content

Commit

Permalink
Merge branch 'main' into jm/update-rl-defaults
Browse files Browse the repository at this point in the history
  • Loading branch information
jmurret authored May 25, 2023
2 parents 999a221 + 7a8f33f commit 9804684
Show file tree
Hide file tree
Showing 24 changed files with 1,166 additions and 122 deletions.
3 changes: 3 additions & 0 deletions .changelog/17424.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:breaking-change
api: The `/v1/health/connect/` and `/v1/health/ingress/` endpoints now immediately return 403 "Permission Denied" errors whenever a token with insufficient `service:read` permissions is provided. Prior to this change, the endpoints returned a success code with an empty result list when a token with insufficient permissions was provided.
```
3 changes: 3 additions & 0 deletions .changelog/17456.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
peering: Fix issue where modifying the list of exported services did not correctly replicate changes for services that exist in a non-default namespace.
```
1 change: 1 addition & 0 deletions .github/workflows/reusable-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ jobs:
working-directory: ${{ matrix.directory }}
version: v1.51.1
args: --build-tags="${{ env.GOTAGS }}" -v
skip-cache: true
- name: Notify Slack
if: ${{ failure() }}
run: .github/scripts/notify_slack.sh
2 changes: 2 additions & 0 deletions .github/workflows/reusable-unit-split.yml
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ jobs:
chmod +x /usr/local/bin/datadog-ci
- name: upload coverage
# do not run on forks
if: ${{ env.DATADOG_API_KEY}}
env:
DD_ENV: ci
run: datadog-ci junit upload --service "$GITHUB_REPOSITORY" ${{env.TEST_RESULTS}}/gotestsum-report.xml
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/reusable-unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ jobs:
chmod +x /usr/local/bin/datadog-ci
- name: upload coverage
# do not run on forks
if: ${{ env.DATADOG_API_KEY}}
env:
DD_ENV: ci
run: datadog-ci junit upload --service "$GITHUB_REPOSITORY" ${{env.TEST_RESULTS}}/gotestsum-report.xml
Expand Down
48 changes: 45 additions & 3 deletions .github/workflows/test-integrations.yml
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ jobs:
chmod +x /usr/local/bin/datadog-ci
- name: upload coverage
# do not run on forks
if: github.event.pull_request.head.repo.full_name == github.repository
env:
DATADOG_API_KEY: "${{ endsWith(github.repository, '-enterprise') && env.DATADOG_API_KEY || secrets.DATADOG_API_KEY }}"
DD_ENV: ci
Expand Down Expand Up @@ -200,18 +202,24 @@ jobs:
chmod +x /usr/local/bin/datadog-ci
- name: upload coverage
# do not run on forks
if: github.event.pull_request.head.repo.full_name == github.repository
env:
DATADOG_API_KEY: "${{ endsWith(github.repository, '-enterprise') && env.DATADOG_API_KEY || secrets.DATADOG_API_KEY }}"
DD_ENV: ci
run: datadog-ci junit upload --service "$GITHUB_REPOSITORY" "${{ env.TEST_RESULTS_DIR }}/gotestsum-report.xml"

- name: upload leader coverage
# do not run on forks
if: github.event.pull_request.head.repo.full_name == github.repository
env:
DATADOG_API_KEY: "${{ endsWith(github.repository, '-enterprise') && env.DATADOG_API_KEY || secrets.DATADOG_API_KEY }}"
DD_ENV: ci
run: datadog-ci junit upload --service "$GITHUB_REPOSITORY" "${{ env.TEST_RESULTS_DIR }}/gotestsum-report-leader.xml"

- name: upload agent coverage
# do not run on forks
if: github.event.pull_request.head.repo.full_name == github.repository
env:
DATADOG_API_KEY: "${{ endsWith(github.repository, '-enterprise') && env.DATADOG_API_KEY || secrets.DATADOG_API_KEY }}"
DD_ENV: ci
Expand Down Expand Up @@ -338,6 +346,8 @@ jobs:
chmod +x /usr/local/bin/datadog-ci
- name: upload coverage
# do not run on forks
if: github.event.pull_request.head.repo.full_name == github.repository
env:
DATADOG_API_KEY: "${{ endsWith(github.repository, '-enterprise') && env.DATADOG_API_KEY || secrets.DATADOG_API_KEY }}"
DD_ENV: ci
Expand Down Expand Up @@ -442,6 +452,8 @@ jobs:
chmod +x /usr/local/bin/datadog-ci
- name: upload coverage
# do not run on forks
if: github.event.pull_request.head.repo.full_name == github.repository
env:
DATADOG_API_KEY: "${{ endsWith(github.repository, '-enterprise') && env.DATADOG_API_KEY || secrets.DATADOG_API_KEY }}"
DD_ENV: ci
Expand All @@ -452,6 +464,9 @@ jobs:
needs:
- setup
- dev-build
permissions:
id-token: write # NOTE: this permission is explicitly required for Vault auth.
contents: read
strategy:
fail-fast: false
matrix:
Expand Down Expand Up @@ -527,10 +542,37 @@ jobs:
COMPOSE_INTERACTIVE_NO_CLI: 1
# tput complains if this isn't set to something.
TERM: ansi
- uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2
# NOTE: ENT specific step as we store secrets in Vault.
- name: Authenticate to Vault
if: ${{ endsWith(github.repository, '-enterprise') }}
id: vault-auth
run: vault-auth

# NOTE: ENT specific step as we store secrets in Vault.
- name: Fetch Secrets
if: ${{ endsWith(github.repository, '-enterprise') }}
id: secrets
uses: hashicorp/vault-action@v2.5.0
with:
name: ${{ env.TEST_RESULTS_ARTIFACT_NAME }}
path: ${{ env.TEST_RESULTS_DIR }}
url: ${{ steps.vault-auth.outputs.addr }}
caCertificate: ${{ steps.vault-auth.outputs.ca_certificate }}
token: ${{ steps.vault-auth.outputs.token }}
secrets: |
kv/data/github/${{ github.repository }}/datadog apikey | DATADOG_API_KEY;
- name: prepare datadog-ci
if: ${{ !endsWith(github.repository, '-enterprise') }}
run: |
curl -L --fail "https://github.com/DataDog/datadog-ci/releases/latest/download/datadog-ci_linux-x64" --output "/usr/local/bin/datadog-ci"
chmod +x /usr/local/bin/datadog-ci
- name: upload coverage
# do not run on forks
if: github.event.pull_request.head.repo.full_name == github.repository
env:
DATADOG_API_KEY: "${{ endsWith(github.repository, '-enterprise') && env.DATADOG_API_KEY || secrets.DATADOG_API_KEY }}"
DD_ENV: ci
run: datadog-ci junit upload --service "$GITHUB_REPOSITORY" $TEST_RESULTS_DIR/results.xml

test-integrations-success:
needs:
Expand Down
4 changes: 2 additions & 2 deletions GNUmakefile
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,13 @@ dev-build:
cp ${MAIN_GOPATH}/bin/consul ./bin/consul


dev-docker-dbg: dev-docker linux dev-build
dev-docker-dbg: dev-docker
@echo "Pulling consul container image - $(CONSUL_IMAGE_VERSION)"
@docker pull consul:$(CONSUL_IMAGE_VERSION) >/dev/null
@echo "Building Consul Development container - $(CONSUL_DEV_IMAGE)"
@# 'consul-dbg:local' tag is needed to run the integration tests
@# 'consul-dev:latest' is needed by older workflows
@docker buildx use default && docker buildx build -t 'consul-dbg:local' -t '$(CONSUL_DEV_IMAGE)' \
@docker buildx use default && docker buildx build -t $(CONSUL_COMPAT_TEST_IMAGE)-dbg:local \
--platform linux/$(GOARCH) \
--build-arg CONSUL_IMAGE_VERSION=$(CONSUL_IMAGE_VERSION) \
--load \
Expand Down
19 changes: 8 additions & 11 deletions agent/consul/health_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,14 @@ func (h *Health) ServiceNodes(args *structs.ServiceSpecificRequest, reply *struc
return err
}

// If we're doing a connect or ingress query, we need read access to the service
// we're trying to find proxies for, so check that.
if args.Connect || args.Ingress {
if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow {
return acl.ErrPermissionDenied
}
}

filter, err := bexpr.CreateFilter(args.Filter, nil, reply.Nodes)
if err != nil {
return err
Expand All @@ -247,17 +255,6 @@ func (h *Health) ServiceNodes(args *structs.ServiceSpecificRequest, reply *struc
return err
}

// If we're doing a connect or ingress query, we need read access to the service
// we're trying to find proxies for, so check that.
if args.Connect || args.Ingress {
// TODO(acl-error-enhancements) Look for ways to percolate this information up to give any feedback to the user.
if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow {
// Return the index here so that the agent cache does not infinitely loop.
reply.Index = index
return nil
}
}

resolvedNodes := nodes
if args.MergeCentralConfig {
for _, node := range resolvedNodes {
Expand Down
7 changes: 3 additions & 4 deletions agent/consul/health_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1125,9 +1125,8 @@ node "foo" {
QueryOptions: structs.QueryOptions{Token: token},
}
var resp structs.IndexedCheckServiceNodes
assert.Nil(t, msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &req, &resp))
assert.ErrorContains(t, msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &req, &resp), "Permission denied")
assert.Len(t, resp.Nodes, 0)
assert.Greater(t, resp.Index, uint64(0))

// List w/ token. This should work since we're requesting "foo", but should
// also only contain the proxies with names that adhere to our ACL.
Expand Down Expand Up @@ -1465,7 +1464,7 @@ func TestHealth_ServiceNodes_Ingress_ACL(t *testing.T) {
ServiceName: "db",
Ingress: true,
}
require.Nil(t, msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &req, &out2))
require.ErrorContains(t, msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &req, &out2), "Permission denied")
require.Len(t, out2.Nodes, 0)

// Requesting a service that is not covered by the token's policy
Expand All @@ -1475,7 +1474,7 @@ func TestHealth_ServiceNodes_Ingress_ACL(t *testing.T) {
Ingress: true,
QueryOptions: structs.QueryOptions{Token: token.SecretID},
}
require.Nil(t, msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &req, &out2))
require.ErrorContains(t, msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &req, &out2), "Permission denied")
require.Len(t, out2.Nodes, 0)

// Requesting service covered by the token's policy
Expand Down
6 changes: 4 additions & 2 deletions agent/grpc-external/services/peerstream/replication.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,11 @@ func (s *Server) handleUpsertExportedServiceList(
exportedServices[snSidecarProxy] = struct{}{}
serviceNames = append(serviceNames, sn)
}
entMeta := structs.NodeEnterpriseMetaInPartition(partition)

_, serviceList, err := s.GetStore().ServiceList(nil, entMeta, peerName)
// Ensure we query services from all namespaces in this partition when we perform
// this query or else we may not propagate updates / deletes correctly.
entMeta := acl.NewEnterpriseMetaWithPartition(partition, acl.WildcardName)
_, serviceList, err := s.GetStore().ServiceList(nil, &entMeta, peerName)
if err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion agent/grpc-external/services/peerstream/stream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1960,7 +1960,7 @@ func processResponse_ExportedServiceUpdates(
localEntMeta acl.EnterpriseMeta,
peerName string,
tests []PeeringProcessResponse_testCase,
) {
) *MutableStatus {
// create a peering in the state store
peerID := "1fabcd52-1d46-49b0-b1d8-71559aee47f5"
require.NoError(t, store.PeeringWrite(31, &pbpeering.PeeringWriteRequest{
Expand Down Expand Up @@ -2041,6 +2041,7 @@ func processResponse_ExportedServiceUpdates(
run(t, tc)
})
}
return mst
}

func Test_processResponse_ExportedServiceUpdates(t *testing.T) {
Expand Down
17 changes: 8 additions & 9 deletions agent/xds/jwt_authn.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ func collectJWTRequirements(i *structs.Intention) []*structs.IntentionJWTProvide
func getPermissionsProviders(p []*structs.IntentionPermission) []*structs.IntentionJWTProvider {
intentionProviders := []*structs.IntentionJWTProvider{}
for _, perm := range p {
if perm.JWT == nil {
continue
}
intentionProviders = append(intentionProviders, perm.JWT.Providers...)
}

Expand All @@ -111,12 +114,8 @@ func buildJWTProviderConfig(p *structs.JWTProviderConfigEntry) (*envoy_http_jwt_
return nil, err
}
envoyCfg.JwksSourceSpecifier = specifier
} else if remote := p.JSONWebKeySet.Remote; remote.URI != "" {
specifier, err := makeRemoteJWKS(remote)
if err != nil {
return nil, err
}
envoyCfg.JwksSourceSpecifier = specifier
} else if remote := p.JSONWebKeySet.Remote; remote != nil && remote.URI != "" {
envoyCfg.JwksSourceSpecifier = makeRemoteJWKS(remote)
} else {
return nil, fmt.Errorf("invalid jwt provider config; missing JSONWebKeySet for provider: %s", p.Name)
}
Expand Down Expand Up @@ -168,7 +167,7 @@ func makeLocalJWKS(l *structs.LocalJWKS, pName string) (*envoy_http_jwt_authn_v3
return specifier, nil
}

func makeRemoteJWKS(r *structs.RemoteJWKS) (*envoy_http_jwt_authn_v3.JwtProvider_RemoteJwks, error) {
func makeRemoteJWKS(r *structs.RemoteJWKS) *envoy_http_jwt_authn_v3.JwtProvider_RemoteJwks {
remote_specifier := envoy_http_jwt_authn_v3.JwtProvider_RemoteJwks{
RemoteJwks: &envoy_http_jwt_authn_v3.RemoteJwks{
HttpUri: &envoy_core_v3.HttpUri{
Expand All @@ -194,7 +193,7 @@ func makeRemoteJWKS(r *structs.RemoteJWKS) (*envoy_http_jwt_authn_v3.JwtProvider
remote_specifier.RemoteJwks.RetryPolicy = p
}

return &remote_specifier, nil
return &remote_specifier
}

func buildJWTRetryPolicy(r *structs.JWKSRetryPolicy) *envoy_core_v3.RetryPolicy {
Expand Down Expand Up @@ -231,7 +230,7 @@ func buildRouteRule(provider *structs.IntentionJWTProvider, perm *structs.Intent
},
}

if perm != nil {
if perm != nil && perm.HTTP != nil {
if perm.HTTP.PathPrefix != "" {
rule.Match.PathSpecifier = &envoy_route_v3.RouteMatch_Prefix{
Prefix: perm.HTTP.PathPrefix,
Expand Down
Loading

0 comments on commit 9804684

Please sign in to comment.