Skip to content

Commit

Permalink
fix code review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Guy Daich <[email protected]>
  • Loading branch information
guydc committed Jan 6, 2025
1 parent 7d004b3 commit fbbfca2
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 26 deletions.
11 changes: 5 additions & 6 deletions internal/provider/kubernetes/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,12 @@ func (r *gatewayAPIReconciler) getExtensionRefFilters(ctx context.Context) ([]un
return resourceItems, nil
}

func (r *gatewayAPIReconciler) getHTTPRouteFilters(ctx context.Context) ([]egv1a1.HTTPRouteFilter, error) {
httpFilterList := new(egv1a1.HTTPRouteFilterList)
if err := r.client.List(ctx, httpFilterList); err != nil {
return nil, fmt.Errorf("failed to list HTTPRouteFilters: %w", err)
func (r *gatewayAPIReconciler) getHTTPRouteFilter(ctx context.Context, name, namespace string) (*egv1a1.HTTPRouteFilter, error) {
hrf := new(egv1a1.HTTPRouteFilter)
if err := r.client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, hrf); err != nil {
return nil, fmt.Errorf("failed to get HTTPRouteFilter: %w", err)
}

return httpFilterList.Items, nil
return hrf, nil
}

// processRouteFilterConfigMapRef adds the referenced ConfigMap in a HTTPRouteFilter
Expand Down
5 changes: 0 additions & 5 deletions internal/provider/kubernetes/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"

egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
"github.com/envoyproxy/gateway/internal/utils"
)

Expand Down Expand Up @@ -61,9 +60,6 @@ type resourceMappings struct {
// The key is the namespaced name, group and kind of the filter and the value is the
// unstructured form of the resource.
extensionRefFilters map[utils.NamespacedNameWithGroupKind]unstructured.Unstructured
// httpRouteFilters is a map of HTTPRouteFilters, where the key is the namespaced name,
// group and kind of the HTTPFilter.
httpRouteFilters map[utils.NamespacedNameWithGroupKind]*egv1a1.HTTPRouteFilter
// Set for storing HTTPRouteExtensions (Envoy Gateway or Custom) NamespacedNames referenced by various
// route rules objects.
allAssociatedHTTPRouteExtensionFilters sets.Set[utils.NamespacedNameWithGroupKind]
Expand Down Expand Up @@ -93,7 +89,6 @@ func newResourceMapping() *resourceMappings {
allAssociatedBackendTLSPolicies: sets.New[string](),
allAssociatedEnvoyExtensionPolicies: sets.New[string](),
extensionRefFilters: map[utils.NamespacedNameWithGroupKind]unstructured.Unstructured{},
httpRouteFilters: map[utils.NamespacedNameWithGroupKind]*egv1a1.HTTPRouteFilter{},
allAssociatedHTTPRouteExtensionFilters: sets.New[utils.NamespacedNameWithGroupKind](),
}
}
17 changes: 3 additions & 14 deletions internal/provider/kubernetes/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,18 +238,6 @@ func (r *gatewayAPIReconciler) processHTTPRoutes(ctx context.Context, gatewayNam
resourceMap *resourceMappings, resourceTree *resource.Resources,
) error {
httpRouteList := &gwapiv1.HTTPRouteList{}
if r.hrfCRDExists {
httpFilters, err := r.getHTTPRouteFilters(ctx)
if err != nil {
return err
}

for i := range httpFilters {
filter := httpFilters[i]
resourceMap.httpRouteFilters[utils.GetNamespacedNameWithGroupKind(&filter)] = &filter
r.processRouteFilterConfigMapRef(ctx, &filter, resourceMap, resourceTree)
}
}

extensionRefFilters, err := r.getExtensionRefFilters(ctx)
if err != nil {
Expand Down Expand Up @@ -416,12 +404,13 @@ func (r *gatewayAPIReconciler) processHTTPRoutes(ctx context.Context, gatewayNam

switch string(filter.ExtensionRef.Kind) {
case egv1a1.KindHTTPRouteFilter:
httpFilter, ok := resourceMap.httpRouteFilters[key]
if !ok {
httpFilter, err := r.getHTTPRouteFilter(ctx, key.Name, key.Namespace)
if err != nil {
r.log.Error(err, "HTTPRouteFilters not found; bypassing rule", "index", i)
continue
}
if !resourceMap.allAssociatedHTTPRouteExtensionFilters.Has(key) {
r.processRouteFilterConfigMapRef(ctx, httpFilter, resourceMap, resourceTree)
resourceMap.allAssociatedHTTPRouteExtensionFilters.Insert(key)
resourceTree.HTTPRouteFilters = append(resourceTree.HTTPRouteFilters, httpFilter)
}
Expand Down
2 changes: 1 addition & 1 deletion release-notes/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ new features: |
# Fixes for bugs identified in previous versions.
bug fixes: |
Fixed a nil pointer error that occurs when a SecurityPolicy refers to a UDS backend
Fixed validation failure when multiple HTTPRoutes refer to the same extension filter
Fixed a validation failure when multiple HTTPRoutes refer to the same extension filter
# Enhancements that improve performance.
performance improvements: |
Expand Down

0 comments on commit fbbfca2

Please sign in to comment.