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 the serving port for idpBuilder configurable #155

Merged
merged 1 commit into from
Feb 26, 2024
Merged

Conversation

nimakaviani
Copy link
Contributor

@nimakaviani nimakaviani commented Feb 24, 2024

  • add a --port flag to configure the default port
  • add go templates directives to override embedded resource manifests
  • template kind cluster config
  • add a sample script to tweak the port for the reference example

closes #139

- add a --port flag to configure the default port
- add go templates directives to override embedded resource manifests
- template kind cluster config
- add a sample script to tweak the port for the reference example

Signed-off-by: Nima Kaviani <[email protected]>
@nimakaviani nimakaviani requested a review from nabuskey February 24, 2024 00:05
@nimakaviani
Copy link
Contributor Author

This PR only dynamically changes the port for the embedded tools and configurations. For external configurations (e.g. the reference implementation) it is left up to the users to make the required modifications to change the port.

For the reference example implementation, the PR also includes a small script that can help users tweak the port prior to deploying the reference implementation. This should help in places such as codespaces where there is a desire to use a port different from the default 8443 port.

Copy link
Collaborator

@nabuskey nabuskey left a comment

Choose a reason for hiding this comment

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

LGTM

@nimakaviani nimakaviani merged commit c7a8afa into main Feb 26, 2024
1 of 2 checks passed
@cmoulliard
Copy link
Contributor

I know that most of the URLs that we are proposing should be exposed using HTTPS/TLS (= 8443 or 443) BUT ideally we should also allow to configure the port 80 like this

Source: https://github.com/snowdrop/k8s-infra/blob/main/kind/kind.sh#L505-L512
  - containerPort: ${INGRESS_80_CONTAINER_PORT}
    hostPort: ${INGRESS_80_CONTAINER_PORT}
    protocol: TCP
    listenAddress: "0.0.0.0"
  - containerPort: ${INGRESS_443_CONTAINER_PORT}
    hostPort: ${INGRESS_443_CONTAINER_PORT}
    protocol: TCP
    listenAddress: "0.0.0.0"

@nimakaviani

@nimakaviani
Copy link
Contributor Author

any particularly usecase @cmoulliard where we cant get away with 443 and require 80 to be open as well? would really like to avoid it unless we really need to.

@cmoulliard
Copy link
Contributor

any particularly usecase @cmoulliard where we cant get away with 443 and require 80 to be open as well?

No. We need both ports and to be able to setup: 80 and 443, etc as ingress's proxy handle HTTP or HTTPS traffic ;-)

@nabuskey nabuskey deleted the config-port branch April 24, 2024 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configurable application port
3 participants