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.

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

Add Support to for providing TLS certificates for Ingress listeners from an SDS source #10903

Merged
merged 21 commits into from
Sep 23, 2021

Conversation

banks
Copy link
Member

@banks banks commented Aug 24, 2021

This builds on #10613.

This PR adds a new low-level feature for Ingress Gateways. It allows operators to specify an external Envoy SDS (Secret Discovery Service) Service and allows loading TLS certificates for listeners.

The PR commits logically work through each package/area that needed updates with relevant tests so may be a cleaner way to view the diff.

Documentation still to-do but the intended usage is as shown in the integration test:

  1. Operator registers the Ingress service with one or more custom static cluster sin bootstrap using the Proxy.Config.envoy_extra_static_clusters_json configuration. This static cluster specifies how to connect to the SDS service(s) and takes care of authenticating the ingress to that SDS service however the SDS integration requires.
  2. The operator can then use new config fields in the ingress-gateway Config Entry to specify which SDS certificate resource names to load (from which clusters) for the whole gateway, each listener or per service.

TODO Before

This is ready for review as it is I think as the code is all here. There are several additional bits that I will either do in this PR or a follow up before this is released:

  • Changelog entry
  • api package updates for Config Entry

TODO In later PR

  • Document the new options in Ingress Gateway Config Entry
  • Document how to use SDS end-to-end (as an "Advanced Topic")
    • I don't think we need a Learn guide here because this is low-level only really needed for folks that want to build a custom integration with an SDS service. We will later provide an opinionated way to do this that will hide all these details from end-users.

@banks banks added type/enhancement Proposed improvement or new feature theme/ingress-gw Track ingress work labels Aug 24, 2021
@github-actions github-actions bot added the theme/envoy/xds Related to Envoy support label Aug 24, 2021
// Return a specific route for this service as it needs a custom FilterChain
// to serve it's custom cert so we should attach it's routes to a separate
// Route too.
return fmt.Sprintf("%s_%s", key.RouteName(), s.ToServiceName().ToServiceID().String()), nil
Copy link
Member Author

@banks banks Aug 25, 2021

Choose a reason for hiding this comment

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

TODO:

  • This causes conflicts in Ent tests because it means that the default ns service web has a route named web in OSS but default/default/web in Ent. causing all golden files to conflict. This probably needs to follow the same logic structs.Upstream.Identifier which omits default values to keep the identifiers consistent between OSS and Ent. Shame we can't easily re-use that method from here with the types available.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in later commit.

@vercel vercel bot temporarily deployed to Preview – consul August 25, 2021 14:40 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging August 25, 2021 14:40 Inactive
@hashicorp-cla
Copy link

hashicorp-cla commented Aug 25, 2021

CLA assistant check
All committers have signed the CLA.

@banks banks force-pushed the feature/mesh-header-manip branch 3 times, most recently from ab93459 to ef64924 Compare August 27, 2021 21:34
Base automatically changed from feature/mesh-header-manip to main September 10, 2021 20:40
@vercel vercel bot temporarily deployed to Preview – consul September 13, 2021 12:36 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging September 13, 2021 12:36 Inactive
@banks
Copy link
Member Author

banks commented Sep 13, 2021

Updated with fixes from Enterprise version.

@vercel vercel bot temporarily deployed to Preview – consul September 14, 2021 15:04 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging September 14, 2021 15:04 Inactive
@vercel vercel bot temporarily deployed to Preview – consul September 23, 2021 10:31 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging September 23, 2021 10:31 Inactive
Copy link
Member

@rboyer rboyer left a comment

Choose a reason for hiding this comment

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

LGTM

@banks banks merged commit 1ecec84 into main Sep 23, 2021
@banks banks deleted the feature/ingress-sds branch September 23, 2021 15:19
@hc-github-team-consul-core
Copy link
Collaborator

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/455247.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/envoy/xds Related to Envoy support theme/ingress-gw Track ingress work type/enhancement Proposed improvement or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants