Skip to content

Commit

Permalink
Merge branch 'main' into f-reloadable-configuration-enable-debug
Browse files Browse the repository at this point in the history
  • Loading branch information
absolutelightning authored Jun 17, 2023
2 parents f5346f3 + 00c8575 commit a0af702
Show file tree
Hide file tree
Showing 69 changed files with 2,633 additions and 338 deletions.
3 changes: 3 additions & 0 deletions .changelog/17755.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
mesh: Stop jwt providers referenced by intentions from being deleted.
```
3 changes: 3 additions & 0 deletions .changelog/17757.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
connect: Improve transparent proxy support for virtual services and failovers.
```
3 changes: 3 additions & 0 deletions .changelog/17759.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
extensions: Improve validation and error feedback for `property-override` builtin Envoy extension
```
3 changes: 3 additions & 0 deletions .changelog/17775.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
connect: Fix issue where changes to service exports were not reflected in proxies.
```
4 changes: 2 additions & 2 deletions .github/workflows/jira-issues.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,14 @@ jobs:

- name: Close ticket
if: ( github.event.action == 'closed' || github.event.action == 'deleted' ) && steps.search.outputs.issue
uses: atlassian/gajira-transition@4749176faf14633954d72af7a44d7f2af01cc92b # v3
uses: atlassian/gajira-transition@38fc9cd61b03d6a53dd35fcccda172fe04b36de3 # v3
with:
issue: ${{ steps.search.outputs.issue }}
transition: "Closed"

- name: Reopen ticket
if: github.event.action == 'reopened' && steps.search.outputs.issue
uses: atlassian/gajira-transition@4749176faf14633954d72af7a44d7f2af01cc92b # v3
uses: atlassian/gajira-transition@38fc9cd61b03d6a53dd35fcccda172fe04b36de3 # v3
with:
issue: ${{ steps.search.outputs.issue }}
transition: "To Do"
4 changes: 2 additions & 2 deletions .github/workflows/jira-pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,14 @@ jobs:

- name: Close ticket
if: ( github.event.action == 'closed' || github.event.action == 'deleted' ) && steps.search.outputs.issue
uses: atlassian/gajira-transition@4749176faf14633954d72af7a44d7f2af01cc92b # v3
uses: atlassian/gajira-transition@38fc9cd61b03d6a53dd35fcccda172fe04b36de3 # v3
with:
issue: ${{ steps.search.outputs.issue }}
transition: "Closed"

- name: Reopen ticket
if: github.event.action == 'reopened' && steps.search.outputs.issue
uses: atlassian/gajira-transition@4749176faf14633954d72af7a44d7f2af01cc92b # v3
uses: atlassian/gajira-transition@38fc9cd61b03d6a53dd35fcccda172fe04b36de3 # v3
with:
issue: ${{ steps.search.outputs.issue }}
transition: "To Do"
21 changes: 14 additions & 7 deletions GNUmakefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

SHELL = bash


GO_MODULES := $(shell find . -name go.mod -exec dirname {} \; | grep -v "proto-gen-rpc-glue/e2e" | sort)

###
Expand Down Expand Up @@ -72,6 +73,7 @@ CI_DEV_DOCKER_NAMESPACE?=hashicorpdev
CI_DEV_DOCKER_IMAGE_NAME?=consul
CI_DEV_DOCKER_WORKDIR?=bin/
################
CONSUL_VERSION?=$(shell cat version/VERSION)

TEST_MODCACHE?=1
TEST_BUILDCACHE?=1
Expand Down Expand Up @@ -188,6 +190,8 @@ dev-docker: linux dev-build
@docker buildx use default && docker buildx build -t 'consul:local' -t '$(CONSUL_DEV_IMAGE)' \
--platform linux/$(GOARCH) \
--build-arg CONSUL_IMAGE_VERSION=$(CONSUL_IMAGE_VERSION) \
--label org.opencontainers.image.version=$(CONSUL_VERSION) \
--label version=$(CONSUL_VERSION) \
--load \
-f $(CURDIR)/build-support/docker/Consul-Dev-Multiarch.dockerfile $(CURDIR)/pkg/bin/

Expand All @@ -208,6 +212,8 @@ remote-docker: check-remote-dev-image-env
@docker buildx use consul-builder && docker buildx build -t '$(REMOTE_DEV_IMAGE)' \
--platform linux/amd64,linux/arm64 \
--build-arg CONSUL_IMAGE_VERSION=$(CONSUL_IMAGE_VERSION) \
--label org.opencontainers.image.version=$(CONSUL_VERSION) \
--label version=$(CONSUL_VERSION) \
--push \
-f $(CURDIR)/build-support/docker/Consul-Dev-Multiarch.dockerfile $(CURDIR)/pkg/bin/

Expand Down Expand Up @@ -351,16 +357,17 @@ lint/%:
@echo "--> Running enumcover ($*)"
@cd $* && GOWORK=off enumcover ./...

# check that the test-container module only imports allowlisted packages
# from the root consul module. Generally we don't want to allow these imports.
# In a few specific instances though it is okay to import test definitions and
# helpers from some of the packages in the root module.
.PHONY: lint-container-test-deps
lint-container-test-deps:
@echo "--> Checking container tests for bad dependencies"
@cd test/integration/consul-container && ( \
found="$$(go list -m all | grep -c '^github.com/hashicorp/consul ')" ; \
if [[ "$$found" != "0" ]]; then \
echo "test/integration/consul-container: This project should not depend on the root consul module" >&2 ; \
exit 1 ; \
fi \
)
@cd test/integration/consul-container && \
$(CURDIR)/build-support/scripts/check-allowed-imports.sh \
github.com/hashicorp/consul \
internal/catalog/catalogtest

# Build the static web ui inside a Docker container. For local testing only; do not commit these assets.
ui: ui-docker
Expand Down
6 changes: 5 additions & 1 deletion agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -4564,7 +4564,11 @@ func (a *Agent) proxyDataSources() proxycfg.DataSources {
sources.ExportedPeeredServices = proxycfgglue.ServerExportedPeeredServices(deps)
sources.FederationStateListMeshGateways = proxycfgglue.ServerFederationStateListMeshGateways(deps)
sources.GatewayServices = proxycfgglue.ServerGatewayServices(deps)
sources.Health = proxycfgglue.ServerHealth(deps, proxycfgglue.ClientHealth(a.rpcClientHealth))
// We do not use this health check currently due to a bug with the way that service exports
// interact with ACLs and the streaming backend. See comments in `proxycfgglue.ServerHealthBlocking`
// for more details.
// sources.Health = proxycfgglue.ServerHealth(deps, proxycfgglue.ClientHealth(a.rpcClientHealth))
sources.Health = proxycfgglue.ServerHealthBlocking(deps, proxycfgglue.ClientHealth(a.rpcClientHealth), server.FSM().State())
sources.HTTPChecks = proxycfgglue.ServerHTTPChecks(deps, a.config.NodeName, proxycfgglue.CacheHTTPChecks(a.cache), a.State)
sources.Intentions = proxycfgglue.ServerIntentions(deps)
sources.IntentionUpstreams = proxycfgglue.ServerIntentionUpstreams(deps)
Expand Down
27 changes: 18 additions & 9 deletions agent/config/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,7 @@ func (b *builder) build() (rt RuntimeConfig, err error) {
AutoEncryptIPSAN: autoEncryptIPSAN,
AutoEncryptAllowTLS: autoEncryptAllowTLS,
AutoConfig: autoConfig,
Cloud: b.cloudConfigVal(c.Cloud),
Cloud: b.cloudConfigVal(c),
ConnectEnabled: connectEnabled,
ConnectCAProvider: connectCAProvider,
ConnectCAConfig: connectCAConfig,
Expand Down Expand Up @@ -1290,6 +1290,10 @@ func (b *builder) validate(rt RuntimeConfig) error {
"1 and 63 bytes.", rt.NodeName)
}

if err := rt.StructLocality().Validate(); err != nil {
return fmt.Errorf("locality is invalid: %s", err)
}

if ipaddr.IsAny(rt.AdvertiseAddrLAN.IP) {
return fmt.Errorf("Advertise address cannot be 0.0.0.0, :: or [::]")
}
Expand Down Expand Up @@ -2541,21 +2545,26 @@ func validateAutoConfigAuthorizer(rt RuntimeConfig) error {
return nil
}

func (b *builder) cloudConfigVal(v *CloudConfigRaw) hcpconfig.CloudConfig {
func (b *builder) cloudConfigVal(v Config) hcpconfig.CloudConfig {
val := hcpconfig.CloudConfig{
ResourceID: os.Getenv("HCP_RESOURCE_ID"),
}
if v == nil {
// Node id might get overriden in setup.go:142
nodeID := stringVal(v.NodeID)
val.NodeID = types.NodeID(nodeID)
val.NodeName = b.nodeName(v.NodeName)

if v.Cloud == nil {
return val
}

val.ClientID = stringVal(v.ClientID)
val.ClientSecret = stringVal(v.ClientSecret)
val.AuthURL = stringVal(v.AuthURL)
val.Hostname = stringVal(v.Hostname)
val.ScadaAddress = stringVal(v.ScadaAddress)
val.ClientID = stringVal(v.Cloud.ClientID)
val.ClientSecret = stringVal(v.Cloud.ClientSecret)
val.AuthURL = stringVal(v.Cloud.AuthURL)
val.Hostname = stringVal(v.Cloud.Hostname)
val.ScadaAddress = stringVal(v.Cloud.ScadaAddress)

if resourceID := stringVal(v.ResourceID); resourceID != "" {
if resourceID := stringVal(v.Cloud.ResourceID); resourceID != "" {
val.ResourceID = resourceID
}
return val
Expand Down
14 changes: 14 additions & 0 deletions agent/config/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
rt.NodeName = "a"
rt.TLS.NodeName = "a"
rt.DataDir = dataDir
rt.Cloud.NodeName = "a"
},
})
run(t, testCase{
Expand All @@ -630,6 +631,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
expected: func(rt *RuntimeConfig) {
rt.NodeID = "a"
rt.DataDir = dataDir
rt.Cloud.NodeID = "a"
},
})
run(t, testCase{
Expand Down Expand Up @@ -1036,6 +1038,13 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
},
},
})
run(t, testCase{
desc: "locality invalid",
args: []string{`-data-dir=` + dataDir},
json: []string{`{"locality": {"zone": "us-west-1a"}}`},
hcl: []string{`locality { zone = "us-west-1a" }`},
expectedErr: "locality is invalid: zone cannot be set without region",
})
run(t, testCase{
desc: "client addr and ports == 0",
args: []string{`-data-dir=` + dataDir},
Expand Down Expand Up @@ -2319,6 +2328,8 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
rt.Cloud = hcpconfig.CloudConfig{
// ID is only populated from env if not populated from other sources.
ResourceID: "env-id",
NodeName: "thehostname",
NodeID: "",
}

// server things
Expand Down Expand Up @@ -2359,6 +2370,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
rt.Cloud = hcpconfig.CloudConfig{
// ID is only populated from env if not populated from other sources.
ResourceID: "file-id",
NodeName: "thehostname",
}

// server things
Expand Down Expand Up @@ -6317,6 +6329,8 @@ func TestLoad_FullConfig(t *testing.T) {
Hostname: "DH4bh7aC",
AuthURL: "332nCdR2",
ScadaAddress: "aoeusth232",
NodeID: types.NodeID("AsUIlw99"),
NodeName: "otlLxGaI",
},
DNSAddrs: []net.Addr{tcpAddr("93.95.95.81:7001"), udpAddr("93.95.95.81:7001")},
DNSARecordLimit: 29907,
Expand Down
4 changes: 3 additions & 1 deletion agent/config/testdata/TestRuntimeConfig_Sanitize.golden
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@
"ManagementToken": "hidden",
"ResourceID": "cluster1",
"ScadaAddress": "",
"TLSConfig": null
"TLSConfig": null,
"NodeID": "",
"NodeName": ""
},
"ConfigEntryBootstrap": [],
"ConnectCAConfig": {},
Expand Down
66 changes: 66 additions & 0 deletions agent/consul/state/config_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,12 @@ func validateProposedConfigEntryInGraph(
case structs.TCPRoute:
case structs.RateLimitIPConfig:
case structs.JWTProvider:
if newEntry == nil && existingEntry != nil {
err := validateJWTProviderIsReferenced(tx, kindName, existingEntry)
if err != nil {
return err
}
}
default:
return fmt.Errorf("unhandled kind %q during validation of %q", kindName.Kind, kindName.Name)
}
Expand Down Expand Up @@ -704,6 +710,66 @@ func getReferencedProviderNames(j *structs.IntentionJWTRequirement, s []*structs
return providerNames
}

// validateJWTProviderIsReferenced iterates over intentions to determine if the provider being
// deleted is referenced by any intention.
//
// This could be an expensive operation based on the number of intentions. We purposely set this to only
// run on delete and don't expect this to be called often.
func validateJWTProviderIsReferenced(tx ReadTxn, kn configentry.KindName, ce structs.ConfigEntry) error {
meta := acl.NewEnterpriseMetaWithPartition(
kn.EnterpriseMeta.PartitionOrDefault(),
acl.DefaultNamespaceName,
)
entry, ok := ce.(*structs.JWTProviderConfigEntry)
if !ok {
return fmt.Errorf("invalid jwt provider config entry: %T", entry)
}

_, ixnEntries, err := configEntriesByKindTxn(tx, nil, structs.ServiceIntentions, &meta)
if err != nil {
return err
}

err = findJWTProviderNameReferences(ixnEntries, entry.Name)
if err != nil {
return err
}

return nil
}

func findJWTProviderNameReferences(entries []structs.ConfigEntry, pName string) error {
errMsg := "cannot delete jwt provider config entry referenced by an intention. Provider name: %s, intention name: %s"
for _, entry := range entries {
ixn, ok := entry.(*structs.ServiceIntentionsConfigEntry)
if !ok {
return fmt.Errorf("type %T is not a service intentions config entry", entry)
}

if ixn.JWT != nil {
for _, prov := range ixn.JWT.Providers {
if prov.Name == pName {
return fmt.Errorf(errMsg, pName, ixn.Name)
}
}
}

for _, s := range ixn.Sources {
for _, perm := range s.Permissions {
if perm.JWT == nil {
continue
}
for _, prov := range perm.JWT.Providers {
if prov.Name == pName {
return fmt.Errorf(errMsg, pName, ixn.Name)
}
}
}
}
}
return nil
}

// This fetches all the jwt-providers config entries and iterates over them
// to validate that any provider referenced exists.
// This is okay because we assume there are very few jwt-providers per partition
Expand Down
Loading

0 comments on commit a0af702

Please sign in to comment.