From 391ed069c46187aafc235e2cc754d8a62a6a9806 Mon Sep 17 00:00:00 2001 From: John Maguire Date: Thu, 27 Apr 2023 11:54:44 -0400 Subject: [PATCH] APIGW: Update how status conditions for certificates are handled (#17115) * Move status condition for invalid certifcate to reference the listener that is using the certificate * Fix where we set the condition status for listeners and certificate refs, added tests * Add changelog --- .changelog/17115.txt | 3 + agent/consul/gateways/controller_gateways.go | 36 +- .../gateways/controller_gateways_test.go | 403 +++++++++++++++++- api/config_entry_status.go | 1 - 4 files changed, 436 insertions(+), 7 deletions(-) create mode 100644 .changelog/17115.txt diff --git a/.changelog/17115.txt b/.changelog/17115.txt new file mode 100644 index 0000000000..8b6a9090db --- /dev/null +++ b/.changelog/17115.txt @@ -0,0 +1,3 @@ +```release-note:improvement +gateway: Change status condition reason for invalid certificate on a listener from "Accepted" to "ResolvedRefs". +``` diff --git a/agent/consul/gateways/controller_gateways.go b/agent/consul/gateways/controller_gateways.go index e399503f35..46651f0043 100644 --- a/agent/consul/gateways/controller_gateways.go +++ b/agent/consul/gateways/controller_gateways.go @@ -241,6 +241,19 @@ func (r *apiGatewayReconciler) reconcileGateway(_ context.Context, req controlle return err } + // set each listener as having valid certs, then overwrite that status condition + // if there are any certificate errors + meta.eachListener(func(listener *structs.APIGatewayListener, bound *structs.BoundAPIGatewayListener) error { + listenerRef := structs.ResourceReference{ + Kind: structs.APIGateway, + Name: meta.BoundGateway.Name, + SectionName: bound.Name, + EnterpriseMeta: meta.BoundGateway.EnterpriseMeta, + } + updater.SetCondition(validCertificate(listenerRef)) + return nil + }) + for ref, err := range certificateErrors { updater.SetCondition(invalidCertificate(ref, err)) } @@ -741,8 +754,14 @@ func (g *gatewayMeta) checkCertificates(store *state.Store) (map[structs.Resourc if err != nil { return err } + listenerRef := structs.ResourceReference{ + Kind: structs.APIGateway, + Name: g.BoundGateway.Name, + SectionName: bound.Name, + EnterpriseMeta: g.BoundGateway.EnterpriseMeta, + } if certificate == nil { - certificateErrors[ref] = errors.New("certificate not found") + certificateErrors[listenerRef] = fmt.Errorf("certificate %q not found", ref.Name) } else { bound.Certificates = append(bound.Certificates, ref) } @@ -840,12 +859,25 @@ func gatewayAccepted() structs.Condition { ) } +// invalidCertificate returns a condition used when a gateway references a +// certificate that does not exist. It takes a ref used to scope the condition +// to a given APIGateway listener. +func validCertificate(ref structs.ResourceReference) structs.Condition { + return structs.NewGatewayCondition( + api.GatewayConditionResolvedRefs, + api.ConditionStatusTrue, + api.GatewayReasonResolvedRefs, + "resolved refs", + ref, + ) +} + // invalidCertificate returns a condition used when a gateway references a // certificate that does not exist. It takes a ref used to scope the condition // to a given APIGateway listener. func invalidCertificate(ref structs.ResourceReference, err error) structs.Condition { return structs.NewGatewayCondition( - api.GatewayConditionAccepted, + api.GatewayConditionResolvedRefs, api.ConditionStatusFalse, api.GatewayListenerReasonInvalidCertificateRef, err.Error(), diff --git a/agent/consul/gateways/controller_gateways_test.go b/agent/consul/gateways/controller_gateways_test.go index 2b304989e4..d88c7a96bd 100644 --- a/agent/consul/gateways/controller_gateways_test.go +++ b/agent/consul/gateways/controller_gateways_test.go @@ -2012,6 +2012,12 @@ func TestAPIGatewayController(t *testing.T) { EnterpriseMeta: *defaultMeta, SectionName: "listener", }), + validCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + EnterpriseMeta: *defaultMeta, + SectionName: "listener", + }), }, }, }, @@ -2104,6 +2110,12 @@ func TestAPIGatewayController(t *testing.T) { EnterpriseMeta: *defaultMeta, SectionName: "listener", }), + validCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + EnterpriseMeta: *defaultMeta, + SectionName: "listener", + }), }, }, }, @@ -2227,6 +2239,12 @@ func TestAPIGatewayController(t *testing.T) { EnterpriseMeta: *defaultMeta, SectionName: "listener", }), + validCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + EnterpriseMeta: *defaultMeta, + SectionName: "listener", + }), }, }, }, @@ -2370,6 +2388,12 @@ func TestAPIGatewayController(t *testing.T) { EnterpriseMeta: *defaultMeta, SectionName: "listener", }), + validCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + EnterpriseMeta: *defaultMeta, + SectionName: "listener", + }), }, }, }, @@ -2511,6 +2535,12 @@ func TestAPIGatewayController(t *testing.T) { EnterpriseMeta: *defaultMeta, SectionName: "listener", }), + validCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + EnterpriseMeta: *defaultMeta, + SectionName: "listener", + }), }, }, }, @@ -2669,6 +2699,16 @@ func TestAPIGatewayController(t *testing.T) { Name: "gateway", SectionName: "tcp-listener", }), + validCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "http-listener", + }), + validCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "tcp-listener", + }), }, }, }, @@ -3013,6 +3053,11 @@ func TestAPIGatewayController(t *testing.T) { Name: "gateway", SectionName: "http-listener", }), + validCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "http-listener", + }), }, }, }, @@ -3287,9 +3332,10 @@ func TestAPIGatewayController(t *testing.T) { Status: structs.Status{ Conditions: []structs.Condition{ invalidCertificate(structs.ResourceReference{ - Kind: structs.InlineCertificate, - Name: "certificate", - }, errors.New("certificate not found")), + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "http-listener", + }, errors.New("certificate \"certificate\" not found")), invalidCertificates(), gatewayListenerNoConflicts(structs.ResourceReference{ Kind: structs.APIGateway, @@ -3360,6 +3406,11 @@ func TestAPIGatewayController(t *testing.T) { Name: "gateway", SectionName: "http-listener", }), + validCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "http-listener", + }), }, }, }, @@ -3378,6 +3429,345 @@ func TestAPIGatewayController(t *testing.T) { }, }, }, + "all-listeners-valid-certificate-refs": { + requests: []controller.Request{{ + Kind: structs.APIGateway, + Name: "gateway", + Meta: acl.DefaultEnterpriseMeta(), + }}, + initialEntries: []structs.ConfigEntry{ + &structs.APIGatewayConfigEntry{ + Kind: structs.APIGateway, + Name: "gateway", + EnterpriseMeta: *defaultMeta, + Listeners: []structs.APIGatewayListener{ + { + Name: "listener-1", + Port: 80, + TLS: structs.APIGatewayTLSConfiguration{ + Certificates: []structs.ResourceReference{{ + Kind: structs.InlineCertificate, + Name: "cert-1", + }}, + }, + }, + { + Name: "listener-2", + Port: 80, + TLS: structs.APIGatewayTLSConfiguration{ + Certificates: []structs.ResourceReference{{ + Kind: structs.InlineCertificate, + Name: "cert-2", + }}, + }, + }, + }, + }, + &structs.InlineCertificateConfigEntry{ + Kind: structs.InlineCertificate, + Name: "cert-1", + EnterpriseMeta: *defaultMeta, + }, + &structs.InlineCertificateConfigEntry{ + Kind: structs.InlineCertificate, + Name: "cert-2", + EnterpriseMeta: *defaultMeta, + }, + }, + finalEntries: []structs.ConfigEntry{ + &structs.APIGatewayConfigEntry{ + Kind: structs.APIGateway, + Name: "gateway", + EnterpriseMeta: *defaultMeta, + Listeners: []structs.APIGatewayListener{ + { + Name: "listener-1", + Port: 80, + TLS: structs.APIGatewayTLSConfiguration{ + Certificates: []structs.ResourceReference{{ + Kind: structs.InlineCertificate, + Name: "cert-1", + }}, + }, + }, + { + Name: "listener-2", + Port: 80, + TLS: structs.APIGatewayTLSConfiguration{ + Certificates: []structs.ResourceReference{{ + Kind: structs.InlineCertificate, + Name: "cert-2", + }}, + }, + }, + }, + Status: structs.Status{ + Conditions: []structs.Condition{ + validCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "listener-1", + }), + validCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "listener-2", + }), + gatewayListenerNoConflicts(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "listener-1", + }), + gatewayListenerNoConflicts(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "listener-2", + }), + gatewayAccepted(), + }, + }, + }, + &structs.BoundAPIGatewayConfigEntry{ + Kind: structs.BoundAPIGateway, + Name: "gateway", + EnterpriseMeta: *defaultMeta, + Listeners: []structs.BoundAPIGatewayListener{ + { + Name: "listener-1", + Certificates: []structs.ResourceReference{ + { + Kind: structs.InlineCertificate, + Name: "cert-1", + }, + }, + }, + { + Name: "listener-2", + Certificates: []structs.ResourceReference{ + { + Kind: structs.InlineCertificate, + Name: "cert-2", + }, + }, + }, + }, + }, + }, + }, + "all-listeners-invalid-certificates": { + requests: []controller.Request{{ + Kind: structs.APIGateway, + Name: "gateway", + Meta: acl.DefaultEnterpriseMeta(), + }}, + initialEntries: []structs.ConfigEntry{ + &structs.APIGatewayConfigEntry{ + Kind: structs.APIGateway, + Name: "gateway", + EnterpriseMeta: *defaultMeta, + Listeners: []structs.APIGatewayListener{ + { + Name: "listener-1", + Port: 80, + TLS: structs.APIGatewayTLSConfiguration{ + Certificates: []structs.ResourceReference{{ + Kind: structs.InlineCertificate, + Name: "missing certificate", + }}, + }, + }, + { + Name: "listener-2", + Port: 80, + TLS: structs.APIGatewayTLSConfiguration{ + Certificates: []structs.ResourceReference{{ + Kind: structs.InlineCertificate, + Name: "another missing certificate", + }}, + }, + }, + }, + }, + }, + finalEntries: []structs.ConfigEntry{ + &structs.APIGatewayConfigEntry{ + Kind: structs.APIGateway, + Name: "gateway", + EnterpriseMeta: *defaultMeta, + Listeners: []structs.APIGatewayListener{ + { + Name: "listener-1", + Port: 80, + TLS: structs.APIGatewayTLSConfiguration{ + Certificates: []structs.ResourceReference{{ + Kind: structs.InlineCertificate, + Name: "missing certificate", + }}, + }, + }, + { + Name: "listener-2", + Port: 80, + TLS: structs.APIGatewayTLSConfiguration{ + Certificates: []structs.ResourceReference{{ + Kind: structs.InlineCertificate, + Name: "another missing certificate", + }}, + }, + }, + }, + Status: structs.Status{ + Conditions: []structs.Condition{ + invalidCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "listener-1", + }, errors.New("certificate \"missing certificate\" not found")), + invalidCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "listener-2", + }, errors.New("certificate \"another missing certificate\" not found")), + invalidCertificates(), + gatewayListenerNoConflicts(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "listener-1", + }), + gatewayListenerNoConflicts(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "listener-2", + }), + }, + }, + }, + &structs.BoundAPIGatewayConfigEntry{ + Kind: structs.BoundAPIGateway, + Name: "gateway", + EnterpriseMeta: *defaultMeta, + Listeners: []structs.BoundAPIGatewayListener{ + {Name: "listener-1"}, + {Name: "listener-2"}, + }, + }, + }, + }, + "mixed-valid-and-invalid-certificate-refs-for-listeners": { + requests: []controller.Request{{ + Kind: structs.APIGateway, + Name: "gateway", + Meta: acl.DefaultEnterpriseMeta(), + }}, + initialEntries: []structs.ConfigEntry{ + &structs.APIGatewayConfigEntry{ + Kind: structs.APIGateway, + Name: "gateway", + EnterpriseMeta: *defaultMeta, + Listeners: []structs.APIGatewayListener{ + { + Name: "invalid-listener", + Port: 80, + TLS: structs.APIGatewayTLSConfiguration{ + Certificates: []structs.ResourceReference{{ + Kind: structs.InlineCertificate, + Name: "missing certificate", + }}, + }, + }, + { + Name: "valid-listener", + Port: 80, + TLS: structs.APIGatewayTLSConfiguration{ + Certificates: []structs.ResourceReference{{ + Kind: structs.InlineCertificate, + Name: "certificate", + }}, + }, + }, + }, + }, + &structs.InlineCertificateConfigEntry{ + Kind: structs.InlineCertificate, + Name: "certificate", + EnterpriseMeta: *defaultMeta, + }, + }, + finalEntries: []structs.ConfigEntry{ + &structs.APIGatewayConfigEntry{ + Kind: structs.APIGateway, + Name: "gateway", + EnterpriseMeta: *defaultMeta, + Listeners: []structs.APIGatewayListener{ + { + Name: "invalid-listener", + Port: 80, + TLS: structs.APIGatewayTLSConfiguration{ + Certificates: []structs.ResourceReference{{ + Kind: structs.InlineCertificate, + Name: "missing certificate", + }}, + }, + }, + { + Name: "valid-listener", + Port: 80, + TLS: structs.APIGatewayTLSConfiguration{ + Certificates: []structs.ResourceReference{{ + Kind: structs.InlineCertificate, + Name: "certificate", + }}, + }, + }, + }, + Status: structs.Status{ + Conditions: []structs.Condition{ + invalidCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "invalid-listener", + }, errors.New("certificate \"missing certificate\" not found")), + validCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "valid-listener", + }), + + invalidCertificates(), + gatewayListenerNoConflicts(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "valid-listener", + }), + gatewayListenerNoConflicts(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "invalid-listener", + }), + }, + }, + }, + &structs.BoundAPIGatewayConfigEntry{ + Kind: structs.BoundAPIGateway, + Name: "gateway", + EnterpriseMeta: *defaultMeta, + Listeners: []structs.BoundAPIGatewayListener{ + { + Name: "valid-listener", + Certificates: []structs.ResourceReference{ + { + Kind: structs.InlineCertificate, + Name: "certificate", + }, + }, + }, + { + Name: "invalid-listener", + }, + }, + }, + }, + }, "updated-gateway-certificates": { requests: []controller.Request{{ Kind: structs.APIGateway, @@ -3443,6 +3833,11 @@ func TestAPIGatewayController(t *testing.T) { Name: "gateway", SectionName: "http-listener", }), + validCertificate(structs.ResourceReference{ + Kind: structs.APIGateway, + Name: "gateway", + SectionName: "http-listener", + }), }, }, }, @@ -3625,7 +4020,7 @@ func TestAPIGatewayController(t *testing.T) { require.NoError(t, err) ppExpected, err := json.MarshalIndent(expectedStatus, "", " ") require.NoError(t, err) - require.True(t, statusEqual, fmt.Sprintf("statuses are unequal: %+v != %+v", string(ppActual), string(ppExpected))) + require.True(t, statusEqual, fmt.Sprintf("statuses are unequal (actual != expected): %+v != %+v", string(ppActual), string(ppExpected))) if bound, ok := controlled.(*structs.BoundAPIGatewayConfigEntry); ok { ppActual, err := json.MarshalIndent(bound, "", " ") require.NoError(t, err) diff --git a/api/config_entry_status.go b/api/config_entry_status.go index dfb97eb4c9..2d16ea0fc4 100644 --- a/api/config_entry_status.go +++ b/api/config_entry_status.go @@ -171,7 +171,6 @@ var validGatewayConditionReasonsMapping = map[GatewayConditionType]map[Condition GatewayReasonAccepted, }, ConditionStatusFalse: { - GatewayListenerReasonInvalidCertificateRef, // TODO: remove this in follow up PR GatewayReasonInvalidCertificates, }, ConditionStatusUnknown: {},