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

Make Concierge server port numbers configurable #888

Merged
merged 4 commits into from
Nov 23, 2021
Merged

Make Concierge server port numbers configurable #888

merged 4 commits into from
Nov 23, 2021

Conversation

cfryanr
Copy link
Member

@cfryanr cfryanr commented Nov 17, 2021

Make several ports configurable to partially address #864.

  • Add aggregatedAPIServerPort to the Concierge's static ConfigMap
    • The aggregated API server now defaults to port 10250 to allow it to work without needing further configuration on private GKE clusters.
  • Add impersonationProxyServerPort to the Concierge's static ConfigMap (still defaults to 8444 as before)
  • Allow both port numbers to be configured to any value within the range 1024 to 65535. It cannot be below 1024 because the container is not running as root.
  • This does not include adding new config knobs to the ytt values file, so while it is possible to change this port without needing to recompile, it is not convenient. This is because it is unlikely that users would need to configure this, and we are waiting for more feedback before promoting it to a first-class configuration option.

Release note:

Makes it possible to change the listening port numbers of the aggregated API server and impersonation proxy, which is typically not necessary but could be used for example on a cluster using host networking where these ports are already consumed by other services. The aggregated API server now defaults to port 10250 to allow it to work without needing further configuration on private GKE clusters.

- Allow the port number to be configured to any value within the
  range 1024 to 65535
- This commit does not include adding new config knobs to the ytt
  values file, so while it is possible to change this port without
  needing to recompile, it is not convenient
@codecov
Copy link

codecov bot commented Nov 17, 2021

Codecov Report

Merging #888 (f28b33b) into main (537f852) will increase coverage by 0.04%.
The diff coverage is 88.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #888      +/-   ##
==========================================
+ Coverage   78.94%   78.99%   +0.04%     
==========================================
  Files         132      132              
  Lines        9491     9510      +19     
==========================================
+ Hits         7493     7512      +19     
+ Misses       1745     1744       -1     
- Partials      253      254       +1     
Impacted Files Coverage Δ
internal/concierge/server/server.go 26.61% <0.00%> (-0.44%) ⬇️
internal/config/concierge/config.go 90.36% <100.00%> (+2.30%) ⬆️
...ntroller/impersonatorconfig/impersonator_config.go 93.42% <100.00%> (+0.01%) ⬆️
...l/localuserauthenticator/localuserauthenticator.go 57.20% <0.00%> (+0.93%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 537f852...f28b33b. Read the comment docs.

- Used to determine on which port the impersonation proxy will bind
- Defaults to 8444, which is the old hard-coded port value
- Allow the port number to be configured to any value within the
  range 1024 to 65535
- This commit does not include adding new config knobs to the ytt
  values file, so while it is possible to change this port without
  needing to recompile, it is not convenient
@cfryanr cfryanr changed the title WIP: make server port numbers customizable Makes Concierge server port numbers configurable Nov 18, 2021
@cfryanr cfryanr changed the title Makes Concierge server port numbers configurable Make Concierge server port numbers configurable Nov 18, 2021
@enj enj enabled auto-merge November 23, 2021 13:30
@enj enj merged commit 5a1de2f into main Nov 23, 2021
@enj enj deleted the customize_ports branch November 23, 2021 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants