Skip to content

Commit

Permalink
Add relationship for nginxproxy
Browse files Browse the repository at this point in the history
  • Loading branch information
@sjberman
sjberman committed Nov 2, 2023
1 parent 54f1c9c commit ec4c46d
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 43 deletions.
5 changes: 4 additions & 1 deletion internal/mode/static/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func StartManager(cfg config.Config) error {
processor := state.NewChangeProcessorImpl(state.ChangeProcessorConfig{
GatewayCtlrName: cfg.GatewayCtlrName,
GatewayClassName: cfg.GatewayClassName,
RelationshipCapturer: relationship.NewCapturerImpl(),
RelationshipCapturer: relationship.NewCapturerImpl(cfg.GatewayCtlrName),
Logger: cfg.Logger.WithName("changeProcessor"),
Validators: validation.Validators{
HTTPFieldsValidator: ngxvalidation.HTTPValidator{},
Expand Down Expand Up @@ -302,6 +302,9 @@ func registerControllers(
},
{
objectType: &ngfAPI.NginxProxy{},
options: []controller.Option{
controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}),
},
},
}

Expand Down
2 changes: 1 addition & 1 deletion internal/mode/static/state/change_processor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl {
{
gvk: extractGVK(&ngfAPI.NginxProxy{}),
store: newObjectStoreMapAdapter(clusterStore.NginxProxies),
trackUpsertDelete: true,
trackUpsertDelete: false,
},
},
)
Expand Down
6 changes: 3 additions & 3 deletions internal/mode/static/state/change_processor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ var _ = Describe("ChangeProcessor", func() {
processor = state.NewChangeProcessorImpl(state.ChangeProcessorConfig{
GatewayCtlrName: controllerName,
GatewayClassName: gcName,
RelationshipCapturer: relationship.NewCapturerImpl(),
RelationshipCapturer: relationship.NewCapturerImpl(controllerName),
Logger: zap.New(),
Validators: createAlwaysValidValidators(),
Scheme: createScheme(),
Expand Down Expand Up @@ -1650,7 +1650,7 @@ var _ = Describe("ChangeProcessor", func() {
processor = state.NewChangeProcessorImpl(state.ChangeProcessorConfig{
GatewayCtlrName: controllerName,
GatewayClassName: gcName,
RelationshipCapturer: relationship.NewCapturerImpl(),
RelationshipCapturer: relationship.NewCapturerImpl(controllerName),
Logger: zap.New(),
Validators: createAlwaysValidValidators(),
EventRecorder: fakeEventRecorder,
Expand Down Expand Up @@ -1823,7 +1823,7 @@ var _ = Describe("ChangeProcessor", func() {
processor = state.NewChangeProcessorImpl(state.ChangeProcessorConfig{
GatewayCtlrName: controllerName,
GatewayClassName: gcName,
RelationshipCapturer: relationship.NewCapturerImpl(),
RelationshipCapturer: relationship.NewCapturerImpl(controllerName),
Logger: zap.New(),
Validators: createAlwaysValidValidators(),
EventRecorder: fakeEventRecorder,
Expand Down
94 changes: 61 additions & 33 deletions internal/mode/static/state/relationship/capturer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/gateway-api/apis/v1beta1"

ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/index"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph"
)
Expand Down Expand Up @@ -46,6 +47,9 @@ type (
}
// namespaces is a collection of namespaces in the system
namespaces map[types.NamespacedName]namespaceCfg
// gatewayClasses is a collection of gatewayclass that reference this controller, with
// their associated ParametersReferences
gatewayClasses map[types.NamespacedName]*v1beta1.ParametersReference
)

func (n namespaceCfg) match() bool {
Expand All @@ -58,17 +62,21 @@ type CapturerImpl struct {
serviceRefCount serviceRefCountMap
gatewayLabelSelectors gatewayLabelSelectorsMap
namespaces namespaces
gatewayClasses gatewayClasses
endpointSliceOwners map[types.NamespacedName]types.NamespacedName
gwCtlrName string
}

// NewCapturerImpl creates a new instance of CapturerImpl.
func NewCapturerImpl() *CapturerImpl {
func NewCapturerImpl(gwCtlrName string) *CapturerImpl {
return &CapturerImpl{
routesToServices: make(routeToServicesMap),
serviceRefCount: make(serviceRefCountMap),
gatewayLabelSelectors: make(gatewayLabelSelectorsMap),
namespaces: make(namespaces),
gatewayClasses: make(gatewayClasses),
endpointSliceOwners: make(map[types.NamespacedName]types.NamespacedName),
gwCtlrName: gwCtlrName,
}
}

Expand All @@ -86,38 +94,7 @@ func (c *CapturerImpl) Capture(obj client.Object) {
}
}
case *v1beta1.Gateway:
var selectors []labels.Selector
for _, listener := range o.Spec.Listeners {
if selector := graph.GetAllowedRouteLabelSelector(listener); selector != nil {
convertedSelector, err := metav1.LabelSelectorAsSelector(selector)
if err == nil {
selectors = append(selectors, convertedSelector)
}
}
}

gatewayName := client.ObjectKeyFromObject(o)
if len(selectors) > 0 {
c.gatewayLabelSelectors[gatewayName] = selectors
for ns, cfg := range c.namespaces {
var gatewayMatches bool
for _, selector := range selectors {
if selector.Matches(labels.Set(cfg.labelMap)) {
gatewayMatches = true
cfg.gateways[gatewayName] = struct{}{}
break
}
}
if !gatewayMatches {
// if gateway was previously referenced by this namespace, clean it up
delete(cfg.gateways, gatewayName)
}
c.namespaces[ns] = cfg
}
} else if _, exists := c.gatewayLabelSelectors[gatewayName]; exists {
// label selectors existed previously for this gateway, so clean up any references to them
c.removeGatewayLabelSelector(gatewayName)
}
c.upsertForGateway(o)
case *v1.Namespace:
nsLabels := o.GetLabels()
gateways := c.matchingGateways(nsLabels)
Expand All @@ -126,6 +103,10 @@ func (c *CapturerImpl) Capture(obj client.Object) {
gateways: gateways,
}
c.namespaces[client.ObjectKeyFromObject(o)] = nsCfg
case *v1beta1.GatewayClass:
if o.Spec.ParametersRef != nil && string(o.Spec.ControllerName) == c.gwCtlrName {
c.gatewayClasses[client.ObjectKeyFromObject(o)] = o.Spec.ParametersRef
}
}
}

Expand All @@ -140,6 +121,8 @@ func (c *CapturerImpl) Remove(resourceType client.Object, nsname types.Namespace
c.removeGatewayLabelSelector(nsname)
case *v1.Namespace:
delete(c.namespaces, nsname)
case *v1beta1.GatewayClass:
delete(c.gatewayClasses, nsname)
}
}

Expand All @@ -154,6 +137,16 @@ func (c *CapturerImpl) Exists(resourceType client.Object, nsname types.Namespace
case *v1.Namespace:
cfg, exists := c.namespaces[nsname]
return exists && cfg.match()
case *ngfAPI.NginxProxy:
for _, ref := range c.gatewayClasses {
if ref.Namespace != nil &&
ref.Group == ngfAPI.GroupName &&
ref.Kind == v1beta1.Kind("NginxProxy") &&
ref.Name == nsname.Name &&
string(*ref.Namespace) == nsname.Namespace {
return true
}
}
}

return false
Expand Down Expand Up @@ -226,6 +219,41 @@ func getBackendServiceNamesFromRoute(hr *v1beta1.HTTPRoute) map[types.Namespaced
return svcNames
}

func (c *CapturerImpl) upsertForGateway(gw *v1beta1.Gateway) {
var selectors []labels.Selector
for _, listener := range gw.Spec.Listeners {
if selector := graph.GetAllowedRouteLabelSelector(listener); selector != nil {
convertedSelector, err := metav1.LabelSelectorAsSelector(selector)
if err == nil {
selectors = append(selectors, convertedSelector)
}
}
}

gatewayName := client.ObjectKeyFromObject(gw)
if len(selectors) > 0 {
c.gatewayLabelSelectors[gatewayName] = selectors
for ns, cfg := range c.namespaces {
var gatewayMatches bool
for _, selector := range selectors {
if selector.Matches(labels.Set(cfg.labelMap)) {
gatewayMatches = true
cfg.gateways[gatewayName] = struct{}{}
break
}
}
if !gatewayMatches {
// if gateway was previously referenced by this namespace, clean it up
delete(cfg.gateways, gatewayName)
}
c.namespaces[ns] = cfg
}
} else if _, exists := c.gatewayLabelSelectors[gatewayName]; exists {
// label selectors existed previously for this gateway, so clean up any references to them
c.removeGatewayLabelSelector(gatewayName)
}
}

// matchingGateways looks through all existing label selectors defined by listeners in a gateway,
// and if any matches are found, returns a map of those gateways
func (c *CapturerImpl) matchingGateways(labelMap map[string]string) map[types.NamespacedName]struct{} {
Expand Down
100 changes: 96 additions & 4 deletions internal/mode/static/state/relationship/capturer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/gateway-api/apis/v1beta1"

ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/index"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/relationship"
Expand Down Expand Up @@ -78,7 +79,7 @@ var _ = Describe("Capturer", func() {

Describe("Capture service relationships for routes", func() {
BeforeEach(OncePerOrdered, func() {
capturer = relationship.NewCapturerImpl()
capturer = relationship.NewCapturerImpl("")
})

assertServiceExists := func(svcName types.NamespacedName, exists bool, refCount int) {
Expand Down Expand Up @@ -252,7 +253,7 @@ var _ = Describe("Capturer", func() {
)

BeforeEach(OncePerOrdered, func() {
capturer = relationship.NewCapturerImpl()
capturer = relationship.NewCapturerImpl("")
})

Describe("Normal cases", Ordered, func() {
Expand Down Expand Up @@ -320,7 +321,7 @@ var _ = Describe("Capturer", func() {
var nsNoLabels, ns *v1.Namespace

BeforeEach(func() {
capturer = relationship.NewCapturerImpl()
capturer = relationship.NewCapturerImpl("")
gw = &v1beta1.Gateway{
ObjectMeta: metav1.ObjectMeta{
Name: "gw",
Expand Down Expand Up @@ -441,9 +442,100 @@ var _ = Describe("Capturer", func() {
})
})
})
Describe("Capture gatewayclass relationships for nginxproxies", Ordered, func() {
BeforeEach(func() {
capturer = relationship.NewCapturerImpl("nginx-ctlr")
})

referencedNP := &ngfAPI.NginxProxy{
ObjectMeta: metav1.ObjectMeta{
Name: "valid",
Namespace: "test",
},
}

nonReferencedNP := &ngfAPI.NginxProxy{
ObjectMeta: metav1.ObjectMeta{
Name: "invalid",
Namespace: "test",
},
}

When("a gatewayclass is created without paramRefs", func() {
It("does not report a relationship", func() {
gc := &v1beta1.GatewayClass{
ObjectMeta: metav1.ObjectMeta{
Name: "gc",
},
Spec: v1beta1.GatewayClassSpec{
ControllerName: "nginx-ctlr",
},
}
capturer.Capture(gc)

Expect(capturer.Exists(referencedNP, client.ObjectKeyFromObject(referencedNP))).To(BeFalse())
})
})
When("a gatewayclass is created with paramRefs but wrong controller name", func() {
It("does not report a relationship", func() {
gc := &v1beta1.GatewayClass{
ObjectMeta: metav1.ObjectMeta{
Name: "gc",
},
Spec: v1beta1.GatewayClassSpec{
ControllerName: "wrong",
ParametersRef: &v1beta1.ParametersReference{
Group: ngfAPI.GroupName,
Kind: v1beta1.Kind("NginxProxy"),
Name: referencedNP.Name,
Namespace: helpers.GetPointer(v1beta1.Namespace(referencedNP.Namespace)),
},
},
}
capturer.Capture(gc)

Expect(capturer.Exists(referencedNP, client.ObjectKeyFromObject(referencedNP))).To(BeFalse())
})
})
When("a gatewayclass is created with paramRefs", func() {
gc := &v1beta1.GatewayClass{
ObjectMeta: metav1.ObjectMeta{
Name: "gc",
},
Spec: v1beta1.GatewayClassSpec{
ControllerName: "nginx-ctlr",
ParametersRef: &v1beta1.ParametersReference{
Group: ngfAPI.GroupName,
Kind: v1beta1.Kind("NginxProxy"),
Name: referencedNP.Name,
Namespace: helpers.GetPointer(v1beta1.Namespace(referencedNP.Namespace)),
},
},
}

When("an NginxProxy is created that isn't referenced", func() {
It("does not report a relationship", func() {
capturer.Capture(gc)
Expect(capturer.Exists(nonReferencedNP, client.ObjectKeyFromObject(nonReferencedNP))).To(BeFalse())
})
})
When("an NginxProxy is created that is referenced", func() {
It("reports a relationship", func() {
capturer.Capture(gc)
Expect(capturer.Exists(referencedNP, client.ObjectKeyFromObject(referencedNP))).To(BeTrue())
})
})
When("a gatewayclass is deleted", func() {
It("does not report a relationship", func() {
capturer.Remove(gc, client.ObjectKeyFromObject(gc))
Expect(capturer.Exists(referencedNP, client.ObjectKeyFromObject(referencedNP))).To(BeFalse())
})
})
})
})
Describe("Edge cases", func() {
BeforeEach(func() {
capturer = relationship.NewCapturerImpl()
capturer = relationship.NewCapturerImpl("")
})
It("Capture does not panic when passed an unsupported resource type", func() {
Expect(func() {
Expand Down
2 changes: 1 addition & 1 deletion internal/mode/static/state/relationship/relationships_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func TestCapturerImpl_DecrementRouteCount(t *testing.T) {
},
}

capturer := NewCapturerImpl()
capturer := NewCapturerImpl("")
svc := types.NamespacedName{Namespace: "test", Name: "svc"}

for _, tc := range testcases {
Expand Down

0 comments on commit ec4c46d

Please sign in to comment.