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

Separate Service Secured And Insecured Ports #1488

Merged
merged 1 commit into from
Jun 16, 2022
Merged

Conversation

ihcsim
Copy link
Contributor

@ihcsim ihcsim commented Jun 16, 2022

Change Overview

This PR separates the secured and insecured service ports of the controller into two separate Helm fields. The securedPort is used if the validating webhook is enabled, per K8s API server requirement. The insecuredPort is used if the validating webhook is disabled.

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Issues

Test Plan

Install Kanister with the validating webhook:

helm install kanister ./helm/kanister-operator --create-namespace --namespace kanister

kubectl -n kanister get svc kanister-kanister-operator -ojsonpath='{.spec.ports}' | jq .                                              
[
  {
    "port": 443,
    "protocol": "TCP",
    "targetPort": 9443
  }
]

Install Kanister without the validating webhook:

helm install kanister ./helm/kanister-operator --create-namespace --namespace kanister --set bpValidatingWebhook.enabled=false

kubectl -n kanister get svc kanister-kanister-operator -ojsonpath='{.spec.ports}' | jq .                                              
[
  {
    "port": 8000,
    "protocol": "TCP",
    "targetPort": 8000
  }
]

Blueprints and actionset creations still work with either setup.

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@github-actions
Copy link

Thanks for submitting this pull request 🎉. The team will review it soon and get back to you.

If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document.

@infraq infraq added this to In Progress in Kanister Jun 16, 2022
@viveksinghggits
Copy link
Contributor

@ihcsim the PR that I raised and just got in, should already be doing this. Or I am missing something.

Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
@ihcsim
Copy link
Contributor Author

ihcsim commented Jun 16, 2022

@viveksinghggits in the previous PR, the controller.service.port will have no effects if the webhook is disabled. So if someone set the service port to 8080, Helm will still override it to 8000.

Kanister automation moved this from In Progress to Reviewer approved Jun 16, 2022
@ihcsim ihcsim added the kueue label Jun 16, 2022
@mergify mergify bot merged commit c33f40f into master Jun 16, 2022
@mergify mergify bot deleted the service-secure-port branch June 16, 2022 21:14
Kanister automation moved this from Reviewer approved to Done Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants