Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Translate HTTPAuthFilter onto HTTPRoute #2836

Merged
merged 19 commits into from
Sep 7, 2023
Merged

Translate HTTPAuthFilter onto HTTPRoute #2836

merged 19 commits into from
Sep 7, 2023

Conversation

t-eckert
Copy link
Contributor

@t-eckert t-eckert commented Aug 24, 2023 •
edited
Loading

Note

This PR merges into #2835

Changes proposed in this PR:

  • Translate JWT filter CRDs to config entry format on HTTP Route.
  • Re-use JWT configuration from API Gateways.
  • Split out filtering into helper functions.

How I've tested this PR:

  • Added unit test

How I expect reviewers to test this PR:

  • Running unit tests

Checklist:

I don't think this needs a CHANGELOG entry because it will get swept into the broader changelog for the feature.

@t-eckert t-eckert changed the base branch from main to NET-4978-add-apigw-jwt-crds August 24, 2023 17:28
Base automatically changed from NET-4978-add-apigw-jwt-crds to apigw-ns-jwt-auth August 25, 2023 20:11
@t-eckert t-eckert force-pushed the gateway-jwt/net-4979-httpauthfilter-to-httproute branch 2 times, most recently from 26d347f to 5a185c9 Compare August 29, 2023 17:40
@t-eckert t-eckert added the theme/api-gateway Related to Consul API Gateway label Aug 29, 2023
jm96441n
jm96441n suggested changes Aug 30, 2023
View reviewed changes
Copy link
Member

@jm96441n jm96441n left a comment

Choose a reason for hiding this comment

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

minor comment so we don't have two implementations of how to translate a struct. otherwise good work!

control-plane/api-gateway/common/translation.go Outdated Show resolved Hide resolved
@t-eckert t-eckert force-pushed the gateway-jwt/net-4979-httpauthfilter-to-httproute branch from df3f9be to 95a82ec Compare August 30, 2023 16:18
@t-eckert t-eckert changed the base branch from apigw-ns-jwt-auth to gateway-policy-translation-NET-4980 August 30, 2023 16:19
@t-eckert t-eckert marked this pull request as ready for review August 31, 2023 21:24
@t-eckert t-eckert added pr/no-changelog PR does not need a corresponding .changelog entry pr/no-backport signals that a PR will not contain a backport label labels Aug 31, 2023
andrewstucki
andrewstucki reviewed Sep 1, 2023
View reviewed changes
control-plane/api-gateway/common/translation_test.go Outdated Show resolved Hide resolved
control-plane/config/crd/bases/consul.hashicorp.com_gatewayclassconfigs.yaml Outdated
description: The value to add to privileged ports ( ports < 1024)
for gateway containers
format: int32
type: integer
Copy link
Contributor

Choose a reason for hiding this comment

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

uh-oh, did the generators not get re-run? these fields seems to be from the addition of some openshift specific stuff, which isn't in-scope in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Potentially. I just ran them for my CRD changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at what changed, I think the previous compilation used the wrong version of KubeBuilder. The version we should be using is 0.8.0. I think the previous change was made with 0.11

Copy link
Member

Choose a reason for hiding this comment

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

can we only include the CRD changes that are necessary for this PR and clean up the rest as part of a PR against main?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I will manually fix these.

control-plane/api-gateway/common/translation.go Outdated Show resolved Hide resolved
@t-eckert t-eckert force-pushed the gateway-jwt/net-4979-httpauthfilter-to-httproute branch from a2e5370 to 85d7698 Compare September 5, 2023 15:13
@t-eckert t-eckert force-pushed the gateway-jwt/net-4979-httpauthfilter-to-httproute branch from 0fc607c to c07eef2 Compare September 6, 2023 20:22
@t-eckert t-eckert force-pushed the gateway-jwt/net-4979-httpauthfilter-to-httproute branch from c07eef2 to 2ca88c4 Compare September 6, 2023 20:26
jm96441n
jm96441n approved these changes Sep 7, 2023
View reviewed changes
Copy link
Member

@jm96441n jm96441n left a comment

Choose a reason for hiding this comment

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

overall LGTM!

@t-eckert t-eckert merged commit 6a9c3ca into gateway-policy-translation-NET-4980 Sep 7, 2023
1 check passed
@t-eckert t-eckert deleted the gateway-jwt/net-4979-httpauthfilter-to-httproute branch September 7, 2023 14:54
sarahalsmiller added a commit that referenced this pull request Sep 7, 2023
@sarahalsmiller
Gateway policy translation NET 4980 (#2835)
0d80055 
* squash

* reset crd-gatewaypolicies

* reset

* reset

* fix lint issues

* fix nil pointer issue

* checkpoint

* change to resourseref key

* update to pull all policies

* add nil checks

* more nil pointer checks for defensice programing

* fix lint issue

* delete comment

* add unit test, fix add function

* Update control-plane/api-gateway/common/translation.go

Co-authored-by: Thomas Eckert <teckert@hashicorp.com>

* Translate HTTPAuthFilter onto HTTPRoute (#2836)

* Add function

* Add RouteAuthFilterKind export

* Add ServicesForRoute function

* Start adding translateHTTPRouteAuth

* Added translation filter to existing filter processing

* Split out formatting into subfunctions

* Remove original function

* Remove ServicesForRoute

* Change httprouteauthfilter to routeauthfilter

* Reuse GatewayJWT type for Routes

* Match Sarah's style for translation functions

* Start adding filter tests

* Wrap up test for filters

* Uncomment other tests

* Use existing v1alpha1 import for group

* Remove old make* function

* Use ConvertSliceFunc

* Fix group in translation_test

* Manually un-diff CRDs

* cleanup

* cleanup

* clean up

* update index function

---------

Co-authored-by: Thomas Eckert <teckert@hashicorp.com>
jm96441n pushed a commit that referenced this pull request Sep 14, 2023
@sarahalsmiller @jm96441n
Gateway policy translation NET 4980 (#2835)
0c84847 
* squash

* reset crd-gatewaypolicies

* reset

* reset

* fix lint issues

* fix nil pointer issue

* checkpoint

* change to resourseref key

* update to pull all policies

* add nil checks

* more nil pointer checks for defensice programing

* fix lint issue

* delete comment

* add unit test, fix add function

* Update control-plane/api-gateway/common/translation.go

Co-authored-by: Thomas Eckert <teckert@hashicorp.com>

* Translate HTTPAuthFilter onto HTTPRoute (#2836)

* Add function

* Add RouteAuthFilterKind export

* Add ServicesForRoute function

* Start adding translateHTTPRouteAuth

* Added translation filter to existing filter processing

* Split out formatting into subfunctions

* Remove original function

* Remove ServicesForRoute

* Change httprouteauthfilter to routeauthfilter

* Reuse GatewayJWT type for Routes

* Match Sarah's style for translation functions

* Start adding filter tests

* Wrap up test for filters

* Uncomment other tests

* Use existing v1alpha1 import for group

* Remove old make* function

* Use ConvertSliceFunc

* Fix group in translation_test

* Manually un-diff CRDs

* cleanup

* cleanup

* clean up

* update index function

---------

Co-authored-by: Thomas Eckert <teckert@hashicorp.com>
jm96441n pushed a commit that referenced this pull request Sep 14, 2023
@sarahalsmiller @jm96441n
Gateway policy translation NET 4980 (#2835)
11d78bb 
* squash

* reset crd-gatewaypolicies

* reset

* reset

* fix lint issues

* fix nil pointer issue

* checkpoint

* change to resourseref key

* update to pull all policies

* add nil checks

* more nil pointer checks for defensice programing

* fix lint issue

* delete comment

* add unit test, fix add function

* Update control-plane/api-gateway/common/translation.go

Co-authored-by: Thomas Eckert <teckert@hashicorp.com>

* Translate HTTPAuthFilter onto HTTPRoute (#2836)

* Add function

* Add RouteAuthFilterKind export

* Add ServicesForRoute function

* Start adding translateHTTPRouteAuth

* Added translation filter to existing filter processing

* Split out formatting into subfunctions

* Remove original function

* Remove ServicesForRoute

* Change httprouteauthfilter to routeauthfilter

* Reuse GatewayJWT type for Routes

* Match Sarah's style for translation functions

* Start adding filter tests

* Wrap up test for filters

* Uncomment other tests

* Use existing v1alpha1 import for group

* Remove old make* function

* Use ConvertSliceFunc

* Fix group in translation_test

* Manually un-diff CRDs

* cleanup

* cleanup

* clean up

* update index function

---------

Co-authored-by: Thomas Eckert <teckert@hashicorp.com>
jm96441n added a commit that referenced this pull request Sep 14, 2023
@jm96441n @sarahalsmiller
[NET-5017] APIGW Status Conditions for RouteAuthFilter and Routes wrt…
65c7002 
… JWT (#2961)

* NET-4978: New CRDs for GW JWT Auth (#2734)

* Added CRDs for gateway policy and httproute auth filter

* Added bats tests

* Correctly configured http route auth filter extension

* Small docs update for operator-sdk usage

* updated docs a bit, added gateway policy CRD

* removed extra crd, updated bats tests

* Added changelog

* Added periods for consistency

* Revert unnecessary changes

* make jwt requirement optional

* Updated jwt config to be optional to allow for other auth types

* Rename HTTPRouteAuthFilter to RouteAuthFilter

* Fix typo for omitempty

* finish httprouteauthfilters rename to routeauthfilters

* Added target reference for gateway policies

* Add period to sentence for linter

* Rename APIGatewayJWT* fields to GatewayJWT* and fixed spots of renaming
of HTTPRouteAuthFilter to RouteAuthFilter

* Gateway policy translation NET 4980 (#2835)

* squash

* reset crd-gatewaypolicies

* reset

* reset

* fix lint issues

* fix nil pointer issue

* checkpoint

* change to resourseref key

* update to pull all policies

* add nil checks

* more nil pointer checks for defensice programing

* fix lint issue

* delete comment

* add unit test, fix add function

* Update control-plane/api-gateway/common/translation.go

Co-authored-by: Thomas Eckert <teckert@hashicorp.com>

* Translate HTTPAuthFilter onto HTTPRoute (#2836)

* Add function

* Add RouteAuthFilterKind export

* Add ServicesForRoute function

* Start adding translateHTTPRouteAuth

* Added translation filter to existing filter processing

* Split out formatting into subfunctions

* Remove original function

* Remove ServicesForRoute

* Change httprouteauthfilter to routeauthfilter

* Reuse GatewayJWT type for Routes

* Match Sarah's style for translation functions

* Start adding filter tests

* Wrap up test for filters

* Uncomment other tests

* Use existing v1alpha1 import for group

* Remove old make* function

* Use ConvertSliceFunc

* Fix group in translation_test

* Manually un-diff CRDs

* cleanup

* cleanup

* clean up

* update index function

---------

Co-authored-by: Thomas Eckert <teckert@hashicorp.com>

* Added status conditions for JWT for auth filters and for routes

* Extract function

* Use more generic error for invalid filter

* Re-run ctrl-manifests with correct controller-generate version

* Clean up from pr review

* gofmt

---------

Co-authored-by: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com>
Co-authored-by: Thomas Eckert <teckert@hashicorp.com>
jm96441n added a commit that referenced this pull request Sep 15, 2023
@jm96441n @sarahalsmiller
APIGW NS JWT Auth (#2962)
2d03f3e 
* NET-4978: New CRDs for GW JWT Auth (#2734)

* Added CRDs for gateway policy and httproute auth filter

* Added bats tests

* Correctly configured http route auth filter extension

* Small docs update for operator-sdk usage

* updated docs a bit, added gateway policy CRD

* removed extra crd, updated bats tests

* Added changelog

* Added periods for consistency

* Revert unnecessary changes

* make jwt requirement optional

* Updated jwt config to be optional to allow for other auth types

* Rename HTTPRouteAuthFilter to RouteAuthFilter

* Fix typo for omitempty

* finish httprouteauthfilters rename to routeauthfilters

* Added target reference for gateway policies

* Add period to sentence for linter

* Rename APIGatewayJWT* fields to GatewayJWT* and fixed spots of renaming
of HTTPRouteAuthFilter to RouteAuthFilter

* Gateway policy translation NET 4980 (#2835)

* squash

* reset crd-gatewaypolicies

* reset

* reset

* fix lint issues

* fix nil pointer issue

* checkpoint

* change to resourseref key

* update to pull all policies

* add nil checks

* more nil pointer checks for defensice programing

* fix lint issue

* delete comment

* add unit test, fix add function

* Update control-plane/api-gateway/common/translation.go

Co-authored-by: Thomas Eckert <teckert@hashicorp.com>

* Translate HTTPAuthFilter onto HTTPRoute (#2836)

* Add function

* Add RouteAuthFilterKind export

* Add ServicesForRoute function

* Start adding translateHTTPRouteAuth

* Added translation filter to existing filter processing

* Split out formatting into subfunctions

* Remove original function

* Remove ServicesForRoute

* Change httprouteauthfilter to routeauthfilter

* Reuse GatewayJWT type for Routes

* Match Sarah's style for translation functions

* Start adding filter tests

* Wrap up test for filters

* Uncomment other tests

* Use existing v1alpha1 import for group

* Remove old make* function

* Use ConvertSliceFunc

* Fix group in translation_test

* Manually un-diff CRDs

* cleanup

* cleanup

* clean up

* update index function

---------

Co-authored-by: Thomas Eckert <teckert@hashicorp.com>

* Added validating webhook for gateway policy (#2912)

* Added validating webhook for gateway policy

* Change denied message to provide more information to the operator

* [APIGW] Add comparison of gateway policies to diffing logic (#2939)

* Fix bug in comparison of gateway policies

* fix fmting

* Added gateway equal test

* Finished adding tests and refactored to use slices convencience
functions

* Reconcile Route Auth Filter changes (#2954)

* Group indices by resource

* Add index for HTTPRoutes referencing RouteAuthFilters

* Add watch for HTTPRoutes referencing RouteAuthFilters

* Add permissions to connect-inject clusterrole

* Compare JWT filters for equality

* Add RouteAuthFilter to resource translator

* [NET-5017] APIGW Status Conditions for Gateway for JWT/Reconcile on JWTProvider Changes (#2950)

* Added watches and status condition on gateway listeners for JWT
validation

* Only append errors if they're non-nil

* Added tests for validating jwt on listener and for adding/retrieving jwt
from resource map

* fix fmting

* Clean up from PR review

* Use two value form of map access

* Rename function

* clean up from PR review

* [NET-5017] APIGW Status Conditions for Gateway Policies (#2955)

* Adding status conditions for gw policy

* Fixed issue where status was not being propagated for policies

* Moved code to correct places

* Revert formatting

* Cleaned up error creation, added validation tests

* Added results tests, updated binding test

* Updates from PR review: clean up comments/appends, use correct
conditions for defaults

* [NET-5017] APIGW Status Conditions for RouteAuthFilter and Routes wrt JWT (#2961)

* NET-4978: New CRDs for GW JWT Auth (#2734)

* Added CRDs for gateway policy and httproute auth filter

* Added bats tests

* Correctly configured http route auth filter extension

* Small docs update for operator-sdk usage

* updated docs a bit, added gateway policy CRD

* removed extra crd, updated bats tests

* Added changelog

* Added periods for consistency

* Revert unnecessary changes

* make jwt requirement optional

* Updated jwt config to be optional to allow for other auth types

* Rename HTTPRouteAuthFilter to RouteAuthFilter

* Fix typo for omitempty

* finish httprouteauthfilters rename to routeauthfilters

* Added target reference for gateway policies

* Add period to sentence for linter

* Rename APIGatewayJWT* fields to GatewayJWT* and fixed spots of renaming
of HTTPRouteAuthFilter to RouteAuthFilter

* Gateway policy translation NET 4980 (#2835)

* squash

* reset crd-gatewaypolicies

* reset

* reset

* fix lint issues

* fix nil pointer issue

* checkpoint

* change to resourseref key

* update to pull all policies

* add nil checks

* more nil pointer checks for defensice programing

* fix lint issue

* delete comment

* add unit test, fix add function

* Update control-plane/api-gateway/common/translation.go

Co-authored-by: Thomas Eckert <teckert@hashicorp.com>

* Translate HTTPAuthFilter onto HTTPRoute (#2836)

* Add function

* Add RouteAuthFilterKind export

* Add ServicesForRoute function

* Start adding translateHTTPRouteAuth

* Added translation filter to existing filter processing

* Split out formatting into subfunctions

* Remove original function

* Remove ServicesForRoute

* Change httprouteauthfilter to routeauthfilter

* Reuse GatewayJWT type for Routes

* Match Sarah's style for translation functions

* Start adding filter tests

* Wrap up test for filters

* Uncomment other tests

* Use existing v1alpha1 import for group

* Remove old make* function

* Use ConvertSliceFunc

* Fix group in translation_test

* Manually un-diff CRDs

* cleanup

* cleanup

* clean up

* update index function

---------

Co-authored-by: Thomas Eckert <teckert@hashicorp.com>

* Added status conditions for JWT for auth filters and for routes

* Extract function

* Use more generic error for invalid filter

* Re-run ctrl-manifests with correct controller-generate version

* Clean up from pr review

* gofmt

---------

Co-authored-by: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com>
Co-authored-by: Thomas Eckert <teckert@hashicorp.com>

* Added changelog

* clean up some renames from httprouteauthfilter -> routeauthfilter

* Fix broken webhook test, added new test

---------

Co-authored-by: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com>
Co-authored-by: Thomas Eckert <teckert@hashicorp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jm96441n jm96441n jm96441n approved these changes

@sarahalsmiller sarahalsmiller Awaiting requested review from sarahalsmiller

@andrewstucki andrewstucki Awaiting requested review from andrewstucki

Assignees
No one assigned
Labels
pr/no-backport signals that a PR will not contain a backport label pr/no-changelog PR does not need a corresponding .changelog entry theme/api-gateway Related to Consul API Gateway
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@t-eckert @andrewstucki @jm96441n @sarahalsmiller