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

feat: default ADDRESS env var on deploy #1837

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

lkingland
Copy link
Member

@lkingland lkingland commented Jun 28, 2023

  • 🎁 Requests that deployed functions listen on all interfaces for requests.

Runtimes should, by default, only listen on the loopback interface for security (they may be func run locally).

The ADDRESS environment variable added here on deploy requests the runtime to listen on all interfaces (0.0.0.0) when deployed, since they will need to listen for client requests and for health readiness/liveness probes.

Additionally, now a user who wishes to securely open their function to only receive requests on a specific interface, such as a WireGuard-encrypted mesh network which presents as a specific interface; that can be achieved by explicitly setting the ADDRESS on their function.

NOTE this env variable is only a suggestion, and it is up to the runtimes to read and use. This is currently respected by scaffolded Go functions (func-runtime-go library), with other runtimes expected to implement as they are updated to support scaffolding.

/kind enhancement

Requests that deployed functions listen on all interfaces by default by
providing an ADDRESS environment variable.
@knative-prow knative-prow bot added kind/enhancement size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 28, 2023
@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Patch coverage: 82.35% and project coverage change: +1.02 🎉

Comparison is base (b6b15f9) 62.51% compared to head (1f62a31) 63.53%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1837      +/-   ##
==========================================
+ Coverage   62.51%   63.53%   +1.02%     
==========================================
  Files         107      107              
  Lines       13732    13749      +17     
==========================================
+ Hits         8585     8736     +151     
+ Misses       4331     4172     -159     
- Partials      816      841      +25     
Flag Coverage Δ
e2e-test 35.31% <82.35%> (+0.05%) ⬆️
e2e-test-oncluster 31.27% <76.47%> (+0.01%) ⬆️
e2e-test-oncluster-runtime 26.65% <0.00%> (?)
e2e-test-runtime-go 25.64% <76.47%> (?)
e2e-test-runtime-node 26.68% <76.47%> (?)
e2e-test-runtime-python 26.68% <76.47%> (?)
e2e-test-runtime-quarkus 26.80% <76.47%> (?)
e2e-test-runtime-springboot 25.87% <76.47%> (?)
e2e-test-runtime-typescript 26.80% <76.47%> (?)
integration-tests 51.69% <82.35%> (+3.56%) ⬆️
unit-tests-macos-latest 49.49% <0.00%> (-0.07%) ⬇️
unit-tests-ubuntu-latest 50.27% <0.00%> (-0.05%) ⬇️
unit-tests-windows-latest 49.50% <0.00%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/knative/deployer.go 64.23% <82.35%> (+0.43%) ⬆️

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@lkingland lkingland requested review from lance, salaboy, zroubalik, aslom and matejvasek and removed request for maximilien and navidshaikh July 4, 2023 03:55
Copy link
Contributor

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

/lgtm

/hold for others

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 4, 2023
@knative-prow
Copy link

knative-prow bot commented Jul 4, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lkingland, zroubalik

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [lkingland,zroubalik]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 4, 2023
Copy link
Member

@lance lance 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, although I wonder if ADDRESS might be too generic and potentially suffer from overlap. Just a thought.

@lance lance added this to the Release 1.11 milestone Jul 12, 2023
@lance
Copy link
Member

lance commented Jul 12, 2023

/unhold

@knative-prow knative-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 12, 2023
@knative-prow knative-prow bot merged commit 4719a43 into knative:main Jul 12, 2023
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/enhancement lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants