Specify primary datacenter for root cert watch #368

nathancoleman · 2022-09-14T18:07:47Z

Fixes #361

Changes proposed in this PR:

In order to successfully watch root certs from a secondary datacenter, we must specify the name of the primary datacenter when setting up the watch. This requires that we accept the primary datacenter name as a flag when starting the controller. The other changes in this PR are just to plumb that value down into the watch initialization.

How I've tested this PR:

Federate between 2 Kubernetes clusters as described in this guide, making sure to enable API gateway in both the primary and secondary clusters
1. apiGateway.enabled: true
2. apiGateway.image: hashicorp/consul-api-gateway:0.4.0
3. global.imageK8S: hashicorpdev/consul-k8s-control-plane:f1a9304a (build from server-acl-init: Create global ACL auth method for API Gateway in secondary dc consul-k8s#1481)
Create a Gateway in the secondary cluster and apply routes targeting services in the same cluster (I use the stack from this Learn guide)
Verify that you can reach these services. For the Learn guide, grab the IP from the Gateway status and visit https://<ip>:8443

Results from completing the above

➜  local git:(main) ✗ # In the secondary federated cluster
➜  local git:(main) ✗ kubectl config current-context
dc2
➜  local git:(main) ✗
➜  local git:(main) ✗ # Successfully deployed gateway
➜  local git:(main) ✗ k get gateway api-gateway -o yaml | yq .status
addresses:
  - type: IPAddress
    value: 34.66.212.71
conditions:
  - lastTransitionTime: "2022-09-15T19:36:12Z"
    message: Ready
    observedGeneration: 1
    reason: Ready
    status: "True"
    type: Ready
  - lastTransitionTime: "2022-09-15T19:36:12Z"
    message: Scheduled
    observedGeneration: 1
    reason: Scheduled
    status: "True"
    type: Scheduled
  - lastTransitionTime: "2022-09-15T19:36:12Z"
    message: InSync
    observedGeneration: 1
    reason: InSync
    status: "True"
    type: InSync
listeners:
  - attachedRoutes: 2
    conditions:
      - lastTransitionTime: "2022-09-15T19:44:01Z"
        message: NoConflicts
        observedGeneration: 1
        reason: NoConflicts
        status: "False"
        type: Conflicted
      - lastTransitionTime: "2022-09-15T19:44:01Z"
        message: Attached
        observedGeneration: 1
        reason: Attached
        status: "False"
        type: Detached
      - lastTransitionTime: "2022-09-15T19:44:01Z"
        message: Ready
        observedGeneration: 1
        reason: Ready
        status: "True"
        type: Ready
      - lastTransitionTime: "2022-09-15T19:44:01Z"
        message: ResolvedRefs
        observedGeneration: 1
        reason: ResolvedRefs
        status: "True"
        type: ResolvedRefs
    name: https
    supportedKinds:
      - group: gateway.networking.k8s.io
        kind: HTTPRoute
➜  local git:(main) ✗
➜  local git:(main) ✗ # Can reach hashicups through the gateway
➜  local git:(main) ✗ curl https://34.66.212.71:8443 --insecure
<!DOCTYPE html><html><head><meta name="viewport" content="width=device-width"/><meta charSet="utf-8"/><script>!function(){try {var d=document.documentElement.classList;d.remove('dark','light','dark','light');var e=localStorage.getItem('theme');if("system"===e||(!e&&true)){var t="(prefers-color-scheme: dark)",m=window.matchMedia(t);m.media!==t||m.matches?d.add('dark'):d.add('light')}else if(e) var x={"dark":"dark","light":"light","autoDark":"dark","autoLight":"light"};d.add(x[e])}catch(e){}}()</script><title>HashiCups - Demo App</title>...

How I expect reviewers to test this PR:

See above

Checklist:

Tests added
CHANGELOG entry added

Run make changelog-entry for guidance in authoring a changelog entry, and
commit the resulting file, which should have a name matching your PR number.
Entries should use imperative present tense (e.g. Add support for...)

nathancoleman · 2022-09-14T18:08:54Z

internal/consul/certmanager.go

@@ -213,7 +216,8 @@ func (c *CertManager) Manage(ctx context.Context) error {
 	c.logger.Trace("running cert manager")

 	rootWatch, err := watch.Parse(map[string]interface{}{
-		"type": "connect_roots",
+		"datacenter": c.primaryDatacenter,


Every other change in this PR is to facilitate this one line where we specify the primary datacenter for our root cert watch. This is necessary for Gateways to work in the secondary datacenter of a federated setup.

Note We'll need a followup change in consul-k8s to provide the primary datacenter as a flag value to the controller

When this flag in not set, does defaulting to empty string for this field infer the current datacenter and match existing behavior? (I'd expect it would be identical because of how Go would initialize the struct.)

Yep! You can see here that watch.Parse() above just calls watch.ParseExempt() which scans each value from the map into a variable on the &Plan{}. In the case of datacenter in particular, the zero value when not included in the map is "", not nil, so the behavior doesn't change when your map includes {"datacenter": ""}.

This is proven out by the conformance test run on this PR which does not set the primary-datacenter flag but still functions as expected.

mikemorris

LGTM pending question about default behavior when the -primary-datacenter flag isn't set and fixing the Consul Enterprise license to get CI tests passing.

nathancoleman · 2022-09-15T20:04:14Z

@mikemorris thoughts on the release note added here?

nathancoleman added 2 commits September 8, 2022 11:59

Add primary dc flag to controller for root cert watch

cccd85b

Add primary dc flag to gateway exec for root cert watch

95b5368

nathancoleman added pr/no-changelog Skip the CI check that requires a changelog entry pr/run-conformance labels Sep 14, 2022

nathancoleman commented Sep 14, 2022

View reviewed changes

nathancoleman linked an issue Sep 14, 2022 that may be closed by this pull request

Controller certificate watch fails in secondary datacenter #361

Closed

nathancoleman requested a review from andrewstucki September 14, 2022 18:22

mikemorris approved these changes Sep 14, 2022

View reviewed changes

Add missing flag declaration for exec process

183c912

nathancoleman marked this pull request as ready for review September 15, 2022 19:46

Add release note

e0863c7

nathancoleman removed the pr/no-changelog Skip the CI check that requires a changelog entry label Sep 15, 2022

nathancoleman merged commit c23f769 into main Sep 15, 2022

nathancoleman deleted the connect_roots_dc branch September 15, 2022 21:09

nathancoleman mentioned this pull request Sep 16, 2022

Populate primary dc flag for APIGW controller in secondary federated dc hashicorp/consul-k8s#1511

Merged

2 tasks

nathancoleman mentioned this pull request Sep 29, 2022

There doesn't appear to be a way to create an API Gateway, or Gateway per cluster in a federated WAN #300

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Specify primary datacenter for root cert watch #368

Specify primary datacenter for root cert watch #368

nathancoleman commented Sep 14, 2022 •

edited by mikemorris

nathancoleman Sep 14, 2022 •

edited

mikemorris Sep 14, 2022 •

edited

nathancoleman Sep 15, 2022 •

edited

mikemorris left a comment

nathancoleman commented Sep 15, 2022

Specify primary datacenter for root cert watch #368

Specify primary datacenter for root cert watch #368

Conversation

nathancoleman commented Sep 14, 2022 • edited by mikemorris

Changes proposed in this PR:

How I've tested this PR:

How I expect reviewers to test this PR:

Checklist:

nathancoleman Sep 14, 2022 • edited

Choose a reason for hiding this comment

mikemorris Sep 14, 2022 • edited

Choose a reason for hiding this comment

nathancoleman Sep 15, 2022 • edited

Choose a reason for hiding this comment

mikemorris left a comment

Choose a reason for hiding this comment

nathancoleman commented Sep 15, 2022

nathancoleman commented Sep 14, 2022 •

edited by mikemorris

nathancoleman Sep 14, 2022 •

edited

mikemorris Sep 14, 2022 •

edited

nathancoleman Sep 15, 2022 •

edited