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(atlantis): add containerPort custom #444

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jullianow
Copy link

what

This change seeks to provide the possibility of defining a port for the container different from the one specified in the service.

why

This change aims to maintain compatibility with scenarios in which the Atlantis administrator wants to deploy a sidecar (using the extraContainers property) in which it will receive the traffic coming from the Service. This is a case, for example, when we want to have an oauth2-proxy responding to requests and acting as a proxy for Atlantis. With this change, this will still be possible for those who need it. By default, the behavior continues to be that the Service's targetPort is the same value for the Atlantis ContainerPort.

@jullianow jullianow requested a review from a team as a code owner December 20, 2024 22:46
@jullianow jullianow force-pushed the containerport-custom branch 3 times, most recently from 4e9ac05 to 14ee0f8 Compare December 20, 2024 23:24
@jullianow jullianow force-pushed the containerport-custom branch from 14ee0f8 to f8a8caa Compare December 20, 2024 23:26
Copy link
Member

@GMartinez-Sisti GMartinez-Sisti left a comment

Choose a reason for hiding this comment

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

Hi @jullianow, thank you for your contrubtion.

Please check the comment below and also:

  • bump chart minor version due to new feature
  • update the comment to reflect the new feature

    -- (int) [optional] Define the port you would like atlantis to run on. Defaults to 4141.

charts/atlantis/values.yaml Outdated Show resolved Hide resolved
@GMartinez-Sisti GMartinez-Sisti added the waiting-on-response Waiting for a response from the user label Dec 21, 2024
@jullianow jullianow force-pushed the containerport-custom branch from f537c79 to 6f38d76 Compare December 23, 2024 11:10
Signed-off-by: Julliano Goncalves <[email protected]>
Copy link
Member

@GMartinez-Sisti GMartinez-Sisti left a comment

Choose a reason for hiding this comment

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

Thanks for the update, just missing some details.

@@ -172,13 +172,16 @@ serviceAccountSecrets: {}
# Default values for atlantis (override as needed).
## -------------------------- ##

# -- (int) [optional] Define the port you would like atlantis to run on. Defaults to 4141.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# -- (int) [optional] Define the port you would like atlantis to run on. Defaults to 4141.
# -- (int) [optional] Define the port you would like atlantis to run on. When 0, the value of service.targetPort is used.

@@ -267,6 +267,15 @@
}
}
},
"containerPort": {
Copy link
Member

Choose a reason for hiding this comment

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

Type needs to be integer alone with default as 0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-response Waiting for a response from the user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants