Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

Specify primary datacenter for root cert watch #368

Merged
merged 4 commits into from
Sep 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/368.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
Fix failing root certificate watch for controller when deployed in secondary federated datacenter.
```
24 changes: 14 additions & 10 deletions internal/commands/exec/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ import (
"github.com/google/uuid"
"github.com/mitchellh/cli"

"github.com/hashicorp/consul-api-gateway/internal/common"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/go-hclog"

"github.com/hashicorp/consul-api-gateway/internal/common"
)

// https://github.com/hashicorp/consul-k8s/blob/24be51c58461e71365ca39f113dae0379f7a1b7c/control-plane/connect-inject/container_init.go#L272-L306
Expand All @@ -39,10 +40,11 @@ type Command struct {
isTest bool

// Consul params
flagConsulHTTPAddress string // Address for Consul HTTP API.
flagConsulHTTPPort int // Port for Consul HTTP communication
flagConsulCACertFile string // Root CA file for Consul
flagConsulXDSPort int // Port for Consul xDS communication
flagConsulHTTPAddress string // Address for Consul HTTP API.
flagConsulHTTPPort int // Port for Consul HTTP communication
flagConsulCACertFile string // Root CA file for Consul
flagConsulXDSPort int // Port for Consul xDS communication
flagConsulPrimaryDatacenter string // Name of the primary datacenter if deploying into a secondary datacenter

// Gateway params
flagGatewayID string // Gateway iD.
Expand Down Expand Up @@ -86,6 +88,7 @@ func (c *Command) init() {
c.flagSet.IntVar(&c.flagConsulHTTPPort, "consul-http-port", 8500, "Port of Consul HTTP server.")
c.flagSet.IntVar(&c.flagConsulXDSPort, "consul-xds-port", 8502, "Port of Consul xDS server.")
c.flagSet.StringVar(&c.flagConsulCACertFile, "consul-ca-cert-file", "", "CA Root file for Consul.")
c.flagSet.StringVar(&c.flagConsulPrimaryDatacenter, "consul-primary-datacenter", "", "Primary datacenter if deploying into a secondary datacenter.")
}
{
// Envoy
Expand Down Expand Up @@ -163,11 +166,12 @@ func (c *Command) Run(args []string) (ret int) {
}

return RunExec(ExecConfig{
Context: c.ctx,
Logger: logger,
LogLevel: c.flagLogLevel,
ConsulClient: consulClient,
ConsulConfig: *cfg,
Context: c.ctx,
Logger: logger,
LogLevel: c.flagLogLevel,
ConsulClient: consulClient,
ConsulConfig: *cfg,
PrimaryDatacenter: c.flagConsulPrimaryDatacenter,
AuthConfig: AuthConfig{
Method: c.flagACLAuthMethod,
Namespace: c.flagAuthMethodNamespace,
Expand Down
23 changes: 13 additions & 10 deletions internal/commands/exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ import (

"golang.org/x/sync/errgroup"

"github.com/hashicorp/consul-api-gateway/internal/consul"
"github.com/hashicorp/consul-api-gateway/internal/envoy"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/go-hclog"

"github.com/hashicorp/consul-api-gateway/internal/consul"
"github.com/hashicorp/consul-api-gateway/internal/envoy"
)

type AuthConfig struct {
Expand Down Expand Up @@ -43,14 +44,15 @@ type EnvoyConfig struct {
}

type ExecConfig struct {
Context context.Context
Logger hclog.Logger
LogLevel string
ConsulClient *api.Client
ConsulConfig api.Config
AuthConfig AuthConfig
GatewayConfig GatewayConfig
EnvoyConfig EnvoyConfig
Context context.Context
Logger hclog.Logger
LogLevel string
ConsulClient *api.Client
ConsulConfig api.Config
AuthConfig AuthConfig
GatewayConfig GatewayConfig
EnvoyConfig EnvoyConfig
PrimaryDatacenter string

// for testing only
isTest bool
Expand Down Expand Up @@ -140,6 +142,7 @@ func RunExec(config ExecConfig) (ret int) {
},
)
options := consul.DefaultCertManagerOptions()
options.PrimaryDatacenter = config.PrimaryDatacenter
options.SDSAddress = config.EnvoyConfig.SDSAddress
options.SDSPort = config.EnvoyConfig.SDSPort
options.Directory = "/certs"
Expand Down
19 changes: 12 additions & 7 deletions internal/commands/server/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ type Command struct {

flagConsulAddress string // Consul server address

flagPrimaryDatacenter string // Primary datacenter, may or may not be the datacenter this controller is running in

flagSDSServerHost string // SDS server host
flagSDSServerPort int // SDS server port
flagMetricsPort int // Port for prometheus metrics
Expand Down Expand Up @@ -70,6 +72,7 @@ func (c *Command) init() {
c.flagSet.StringVar(&c.flagCASecret, "ca-secret", "", "CA Secret for Consul server.")
c.flagSet.StringVar(&c.flagCASecretNamespace, "ca-secret-namespace", "default", "CA Secret namespace for Consul server.")
c.flagSet.StringVar(&c.flagConsulAddress, "consul-address", "", "Consul Address.")
c.flagSet.StringVar(&c.flagPrimaryDatacenter, "primary-datacenter", "", "Name of the primary Consul datacenter")
c.flagSet.StringVar(&c.flagSDSServerHost, "sds-server-host", defaultSDSServerHost, "SDS Server Host.")
c.flagSet.StringVar(&c.flagK8sContext, "k8s-context", "", "Kubernetes context to use.")
c.flagSet.StringVar(&c.flagK8sNamespace, "k8s-namespace", "", "Kubernetes namespace to use.")
Expand Down Expand Up @@ -119,6 +122,7 @@ func (c *Command) Run(args []string) int {
cfg.SDSServerHost = c.flagSDSServerHost
cfg.SDSServerPort = c.flagSDSServerPort
cfg.Namespace = c.flagK8sNamespace
cfg.PrimaryDatacenter = c.flagPrimaryDatacenter

consulCfg := api.DefaultConfig()
if c.flagCAFile != "" {
Expand Down Expand Up @@ -164,13 +168,14 @@ func (c *Command) Run(args []string) int {
}

return RunServer(ServerConfig{
Context: context.Background(),
Logger: logger,
ConsulConfig: consulCfg,
K8sConfig: cfg,
ProfilingPort: c.flagPprofPort,
MetricsPort: c.flagMetricsPort,
isTest: c.isTest,
Context: context.Background(),
Logger: logger,
ConsulConfig: consulCfg,
K8sConfig: cfg,
ProfilingPort: c.flagPprofPort,
MetricsPort: c.flagMetricsPort,
PrimaryDatacenter: c.flagPrimaryDatacenter,
isTest: c.isTest,
})
}

Expand Down
15 changes: 9 additions & 6 deletions internal/commands/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@ import (
)

type ServerConfig struct {
Context context.Context
Logger hclog.Logger
ConsulConfig *api.Config
K8sConfig *k8s.Config
ProfilingPort int
MetricsPort int
Context context.Context
Logger hclog.Logger
ConsulConfig *api.Config
K8sConfig *k8s.Config
ProfilingPort int
MetricsPort int
PrimaryDatacenter string

// for testing only
isTest bool
Expand Down Expand Up @@ -72,6 +73,8 @@ func RunServer(config ServerConfig) int {
controller.SetStore(store)

options := consul.DefaultCertManagerOptions()
options.PrimaryDatacenter = config.PrimaryDatacenter

certManager := consul.NewCertManager(
config.Logger.Named("cert-manager"),
consulClient,
Expand Down
38 changes: 21 additions & 17 deletions internal/consul/certmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,10 @@ func init() {

// CertManagerOptions contains the optional configuration used to initialize a CertManager.
type CertManagerOptions struct {
Directory string
SDSAddress string
SDSPort int
Directory string
PrimaryDatacenter string
SDSAddress string
SDSPort int
}

// DefaultCertManagerOptions returns the default options for a CertManager instance.
Expand All @@ -97,11 +98,12 @@ type CertManager struct {
consul *api.Client
logger hclog.Logger

service string
directory string
configDirectory string // only used for testing
sdsAddress string
sdsPort int
service string
directory string
configDirectory string // only used for testing
primaryDatacenter string
sdsAddress string
sdsPort int

lock sync.RWMutex

Expand Down Expand Up @@ -130,14 +132,15 @@ func NewCertManager(logger hclog.Logger, consul *api.Client, service string, opt
options = DefaultCertManagerOptions()
}
manager := &CertManager{
consul: consul,
logger: logger,
sdsAddress: options.SDSAddress,
sdsPort: options.SDSPort,
service: service,
configDirectory: options.Directory,
directory: options.Directory,
initializeSignal: make(chan struct{}),
consul: consul,
logger: logger,
primaryDatacenter: options.PrimaryDatacenter,
sdsAddress: options.SDSAddress,
sdsPort: options.SDSPort,
service: service,
configDirectory: options.Directory,
directory: options.Directory,
initializeSignal: make(chan struct{}),
}
manager.writeCerts = manager.persist
return manager
Expand Down Expand Up @@ -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,
Copy link
Member Author

@nathancoleman nathancoleman Sep 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@mikemorris mikemorris Sep 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Copy link
Member Author

@nathancoleman nathancoleman Sep 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"type": "connect_roots",
})
if err != nil {
return err
Expand Down
66 changes: 38 additions & 28 deletions internal/k8s/builder/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,13 @@ func filterAnnotations(annotations map[string]string, allowed []string) map[stri
}

type GatewayDeploymentBuilder struct {
gateway *gwv1beta1.Gateway
gwConfig *v1alpha1.GatewayClassConfig
sdsHost string
sdsPort int
consulCAData string
consulGatewayNamespace string
gateway *gwv1beta1.Gateway
gwConfig *v1alpha1.GatewayClassConfig
sdsHost string
sdsPort int
consulCAData string
consulGatewayNamespace string
consulPrimaryDatacenter string
}

func NewGatewayDeployment(gw *gwv1beta1.Gateway) *GatewayDeploymentBuilder {
Expand All @@ -109,6 +110,10 @@ func (b *GatewayDeploymentBuilder) WithConsulGatewayNamespace(namespace string)
b.consulGatewayNamespace = namespace
}

func (b *GatewayDeploymentBuilder) WithPrimaryConsulDatacenter(datacenter string) {
b.consulPrimaryDatacenter = datacenter
}

func (b *GatewayDeploymentBuilder) Validate() error {
if b.gwConfig == nil {
return fmt.Errorf("GatewayClassConfig must be set")
Expand Down Expand Up @@ -262,16 +267,17 @@ func (b *GatewayDeploymentBuilder) podSpec() corev1.PodSpec {
func (b *GatewayDeploymentBuilder) execCommand() []string {
// Render the command
data := gwContainerCommandData{
ACLAuthMethod: b.gwConfig.Spec.ConsulSpec.AuthSpec.Method,
ConsulHTTPAddr: orDefault(b.gwConfig.Spec.ConsulSpec.Address, defaultConsulAddress),
ConsulHTTPPort: orDefaultIntString(b.gwConfig.Spec.ConsulSpec.PortSpec.HTTP, defaultConsulHTTPPort),
ConsulGRPCPort: orDefaultIntString(b.gwConfig.Spec.ConsulSpec.PortSpec.GRPC, defaultConsulXDSPort),
LogLevel: orDefault(b.gwConfig.Spec.LogLevel, defaultLogLevel),
GatewayHost: "$(IP)",
GatewayName: b.gateway.Name,
GatewayNamespace: b.consulGatewayNamespace,
SDSHost: b.sdsHost,
SDSPort: b.sdsPort,
ACLAuthMethod: b.gwConfig.Spec.ConsulSpec.AuthSpec.Method,
ConsulHTTPAddr: orDefault(b.gwConfig.Spec.ConsulSpec.Address, defaultConsulAddress),
ConsulHTTPPort: orDefaultIntString(b.gwConfig.Spec.ConsulSpec.PortSpec.HTTP, defaultConsulHTTPPort),
ConsulGRPCPort: orDefaultIntString(b.gwConfig.Spec.ConsulSpec.PortSpec.GRPC, defaultConsulXDSPort),
LogLevel: orDefault(b.gwConfig.Spec.LogLevel, defaultLogLevel),
GatewayHost: "$(IP)",
GatewayName: b.gateway.Name,
GatewayNamespace: b.consulGatewayNamespace,
PrimaryDatacenter: b.consulPrimaryDatacenter,
SDSHost: b.sdsHost,
SDSPort: b.sdsPort,
}
if b.requiresCA() {
data.ConsulCAFile = consulCALocalFile
Expand Down Expand Up @@ -349,18 +355,19 @@ func (b *GatewayDeploymentBuilder) requiresCA() bool {
}

type gwContainerCommandData struct {
ConsulCAFile string
ConsulCAData string
ConsulHTTPAddr string
ConsulHTTPPort string
ConsulGRPCPort string
ACLAuthMethod string
LogLevel string
GatewayHost string
GatewayName string
GatewayNamespace string
SDSHost string
SDSPort int
ConsulCAFile string
ConsulCAData string
ConsulHTTPAddr string
ConsulHTTPPort string
ConsulGRPCPort string
ACLAuthMethod string
LogLevel string
GatewayHost string
GatewayName string
GatewayNamespace string
PrimaryDatacenter string
SDSHost string
SDSPort int
}

// gwContainerCommandTpl is the template for the command executed by
Expand All @@ -385,6 +392,9 @@ exec /bootstrap/consul-api-gateway exec -log-json \
-consul-xds-port {{ .ConsulGRPCPort }} \
{{- if .ACLAuthMethod }}
-acl-auth-method {{ .ACLAuthMethod }} \
{{- end }}
{{- if .PrimaryDatacenter }}
-consul-primary-datacenter {{ .PrimaryDatacenter }} \
{{- end }}
-envoy-bootstrap-path /bootstrap/envoy.json \
-envoy-sds-address {{ .SDSHost }} \
Expand Down
2 changes: 2 additions & 0 deletions internal/k8s/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ type Kubernetes struct {

type Config struct {
CACert string
PrimaryDatacenter string
SDSServerHost string
SDSServerPort int
MetricsBindAddr string
Expand Down Expand Up @@ -154,6 +155,7 @@ func (k *Kubernetes) Start(ctx context.Context) error {
Client: gwClient,
Consul: k.consul,
ConsulCA: k.config.CACert,
PrimaryDatacenter: k.config.PrimaryDatacenter,
SDSHost: k.config.SDSServerHost,
SDSPort: k.config.SDSServerPort,
Logger: k.logger.Named("Reconciler"),
Expand Down
Loading