Skip to content
Draft
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
7 changes: 4 additions & 3 deletions internal/controller/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,10 @@ func StartManager(cfg config.Config) error {
GenericValidator: genericValidator,
PolicyValidator: policyManager,
},
EventRecorder: recorder,
MustExtractGVK: mustExtractGVK,
PlusSecrets: plusSecrets,
EventRecorder: recorder,
MustExtractGVK: mustExtractGVK,
PlusSecrets: plusSecrets,
ExperimentalFeatures: cfg.ExperimentalFeatures,
})

var handlerCollector handlerMetricsCollector = collectors.NewControllerNoopCollector()
Expand Down
2 changes: 2 additions & 0 deletions internal/controller/nginx/config/base_http_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ var baseHTTPTemplate = gotemplate.Must(gotemplate.New("baseHttp").Parse(baseHTTP

type httpConfig struct {
DNSResolver *dataplane.DNSResolverConfig
GatewaySecretID dataplane.SSLKeyPairID
Includes []shared.Include
NginxReadinessProbePort int32
IPFamily shared.IPFamily
Expand All @@ -27,6 +28,7 @@ func executeBaseHTTPConfig(conf dataplane.Configuration) []executeResult {
NginxReadinessProbePort: conf.BaseHTTPConfig.NginxReadinessProbePort,
IPFamily: getIPFamily(conf.BaseHTTPConfig),
DNSResolver: conf.BaseHTTPConfig.DNSResolver,
GatewaySecretID: conf.BaseHTTPConfig.GatewaySecretID,
}

results := make([]executeResult, 0, len(includes)+1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ server {
}
}

{{- if $.GatewaySecretID }}
# Gateway Certificate
proxy_ssl_certificate /etc/nginx/secrets/{{ $.GatewaySecretID }}.pem;
proxy_ssl_certificate_key /etc/nginx/secrets/{{ $.GatewaySecretID }}.pem;
{{- end }}

{{ range $i := .Includes -}}
include {{ $i.Name }};
{{ end -}}
Expand Down
46 changes: 46 additions & 0 deletions internal/controller/nginx/config/base_http_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,3 +284,49 @@ func TestExecuteBaseHttp_DNSResolver(t *testing.T) {
})
}
}

func TestExecuteBaseHttp_GatewaySecretID(t *testing.T) {
t.Parallel()

tests := []struct {
name string
expectedConfig string
conf dataplane.Configuration
}{
{
name: "with GatewaySecretID",
conf: dataplane.Configuration{
BaseHTTPConfig: dataplane.BaseHTTPConfig{
GatewaySecretID: "client-secret",
},
},
expectedConfig: "proxy_ssl_certificate /etc/nginx/secrets/client-secret.pem;" +
"\nproxy_ssl_certificate_key /etc/nginx/secrets/client-secret.pem;",
},
{
name: "without GatewaySecretID",
conf: dataplane.Configuration{
BaseHTTPConfig: dataplane.BaseHTTPConfig{
GatewaySecretID: "",
},
},
expectedConfig: "",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
g := NewWithT(t)

res := executeBaseHTTPConfig(test.conf)
g.Expect(res).To(HaveLen(1))

httpConfig := string(res[0].data)

if test.expectedConfig != "" {
g.Expect(httpConfig).To(ContainSubstring(test.expectedConfig))
}
})
}
}
3 changes: 3 additions & 0 deletions internal/controller/state/change_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ type ChangeProcessorConfig struct {
GatewayCtlrName string
// GatewayClassName is the name of the GatewayClass resource.
GatewayClassName string
// ExperimentalFeatures indicates whether experimental features are enabled.
ExperimentalFeatures bool
}

// ChangeProcessorImpl is an implementation of ChangeProcessor.
Expand Down Expand Up @@ -269,6 +271,7 @@ func (c *ChangeProcessorImpl) Process() *graph.Graph {
c.cfg.PlusSecrets,
c.cfg.Validators,
c.cfg.Logger,
c.cfg.ExperimentalFeatures,
)

return c.latestGraph
Expand Down
29 changes: 29 additions & 0 deletions internal/controller/state/conditions/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,14 @@ const (
// parametersRef resource is invalid.
GatewayReasonParamsRefInvalid v1.GatewayConditionReason = "ParametersRefInvalid"

// GatewayReasonSecretRefInvalid is used with the "GatewayResolvedRefs" condition when the
// secretRef resource is invalid.
GatewayReasonSecretRefInvalid v1.GatewayConditionReason = "SecretRefInvalid"

// GatewayReasonSecretRefNotPermitted is used with the "GatewayResolvedRefs" condition when the
// secretRef resource is not permitted by any ReferenceGrant.
GatewayReasonSecretRefNotPermitted v1.GatewayConditionReason = "SecretRefNotPermitted"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these available or will these be available to import from the gateway api?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, these weren't available on gateway api only reason related to parametersRef so didn't want to use those to avoid confusion


// PolicyReasonAncestorLimitReached is used with the "PolicyAccepted" condition when a policy
// cannot be applied because the ancestor status list has reached the maximum size of 16.
PolicyReasonAncestorLimitReached v1alpha2.PolicyConditionReason = "AncestorLimitReached"
Expand Down Expand Up @@ -297,6 +305,27 @@ func NewGatewayClassUnsupportedVersion(recommendedVersion string) []Condition {
}
}

// NewGatewaySecretRefNotPermitted returns Condition that indicates that the Gateway references a TLS secret that is not
// permitted by any ReferenceGrant.
func NewGatewaySecretRefNotPermitted(msg string) Condition {
return Condition{
Type: string(GatewayReasonResolvedRefs),
Status: metav1.ConditionFalse,
Reason: string(GatewayReasonSecretRefNotPermitted),
Message: msg,
}
}

// NewGatewaySecretRefInvalid returns Condition that indicates that the Gateway references a TLS secret that is invalid.
func NewGatewaySecretRefInvalid(msg string) Condition {
return Condition{
Type: string(GatewayReasonResolvedRefs),
Status: metav1.ConditionFalse,
Reason: string(GatewayReasonSecretRefInvalid),
Message: msg,
}
}

Comment on lines +310 to +328
Copy link
Contributor Author

Choose a reason for hiding this comment

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

currently working on the bug related to marking HTTPRoute invalid leading to issues and I am wondering if this is still valid? Should we mark the gateway invalid? its not syntactically wrong for it to be marked false. We just won't reference the secret in the our configuration so I would like to change this to Accepted: true but i'd like to get second opinions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The Gateway API community may be able to clarify for sure, but I'm fairly confident this should be Accepted: false. If a user wants a secure connection, we definitely shouldn't ignore that if the Secret is invalid. That would defeat the purpose of a secure connection if we just default to plaintext and insecure if their certs aren't able to be used.

// NewGatewayClassConflict returns a Condition that indicates that the GatewayClass is not accepted
// due to a conflict with another GatewayClass.
func NewGatewayClassConflict() Condition {
Expand Down
43 changes: 33 additions & 10 deletions internal/controller/state/dataplane/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,10 @@ func BuildConfiguration(
gateway,
serviceResolver,
g.ReferencedServices,
baseHTTPConfig.IPFamily),
baseHTTPConfig.IPFamily,
),
BackendGroups: backendGroups,
SSLKeyPairs: buildSSLKeyPairs(g.ReferencedSecrets, gateway.Listeners),
SSLKeyPairs: buildSSLKeyPairs(g.ReferencedSecrets, gateway),
CertBundles: buildCertBundles(
buildRefCertificateBundles(g.ReferencedSecrets, g.ReferencedCaCertConfigMaps),
backendGroups,
Expand Down Expand Up @@ -248,14 +249,14 @@ func buildStreamUpstreams(
}

// buildSSLKeyPairs builds the SSLKeyPairs from the Secrets. It will only include Secrets that are referenced by
// valid listeners, so that we don't include unused Secrets in the configuration of the data plane.
// valid gateway and its listeners, so that we don't include unused Secrets in the configuration of the data plane.
func buildSSLKeyPairs(
secrets map[types.NamespacedName]*graph.Secret,
listeners []*graph.Listener,
gateway *graph.Gateway,
) map[SSLKeyPairID]SSLKeyPair {
keyPairs := make(map[SSLKeyPairID]SSLKeyPair)

for _, l := range listeners {
for _, l := range gateway.Listeners {
if l.Valid && l.ResolvedSecret != nil {
id := generateSSLKeyPairID(*l.ResolvedSecret)
secret := secrets[*l.ResolvedSecret]
Expand All @@ -268,6 +269,15 @@ func buildSSLKeyPairs(
}
}

if gateway.Valid && gateway.SecretRef != nil {
id := generateSSLKeyPairID(*gateway.SecretRef)
secret := secrets[*gateway.SecretRef]
keyPairs[id] = SSLKeyPair{
Cert: secret.CertBundle.Cert.TLSCert,
Key: secret.CertBundle.Cert.TLSPrivateKey,
}
}

return keyPairs
}

Expand Down Expand Up @@ -1019,6 +1029,10 @@ func buildBaseHTTPConfig(
NginxReadinessProbePort: DefaultNginxReadinessProbePort,
}

if gateway.Valid && gateway.SecretRef != nil {
baseConfig.GatewaySecretID = generateSSLKeyPairID(*gateway.SecretRef)
}

// safe to access EffectiveNginxProxy since we only call this function when the Gateway is not nil.
np := gateway.EffectiveNginxProxy
if np == nil {
Expand All @@ -1042,8 +1056,20 @@ func buildBaseHTTPConfig(
}
}

if port := getNginxReadinessProbePort(np); port != 0 {
baseConfig.NginxReadinessProbePort = port
}

baseConfig.RewriteClientIPSettings = buildRewriteClientIPConfig(np.RewriteClientIP)

baseConfig.DNSResolver = buildDNSResolverConfig(np.DNSResolver)

return baseConfig
}

func getNginxReadinessProbePort(np *graph.EffectiveNginxProxy) int32 {
var port int32

if np.Kubernetes != nil {
var containerSpec *ngfAPIv1alpha2.ContainerSpec
if np.Kubernetes.Deployment != nil {
Expand All @@ -1052,13 +1078,10 @@ func buildBaseHTTPConfig(
containerSpec = &np.Kubernetes.DaemonSet.Container
}
if containerSpec != nil && containerSpec.ReadinessProbe != nil && containerSpec.ReadinessProbe.Port != nil {
baseConfig.NginxReadinessProbePort = *containerSpec.ReadinessProbe.Port
port = *containerSpec.ReadinessProbe.Port
}
}

baseConfig.DNSResolver = buildDNSResolverConfig(np.DNSResolver)

return baseConfig
return port
}

// buildBaseStreamConfig generates the base stream context config that should be applied to all stream servers.
Expand Down
Loading