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

chore: delete OAuth support for CLI #956

Merged
merged 1 commit into from
Nov 6, 2024
Merged

Conversation

rangoo94
Copy link
Member

@rangoo94 rangoo94 commented Nov 6, 2024

Pull request description

Checklist (choose whats happened)

  • breaking change! (describe)
  • tested locally
  • tested on cluster
  • added new dependencies
  • updated the docs
  • added a test

@rangoo94 rangoo94 requested review from a team as code owners November 6, 2024 14:42
@rangoo94 rangoo94 merged commit c3ecf77 into main Nov 6, 2024
1 check passed
@rangoo94 rangoo94 deleted the dawid/chore/delete-oauth branch November 6, 2024 14:56
@thradec
Copy link

thradec commented Nov 11, 2024

Hi, we were using direct cli access without oauth and now the ingress definition is gone. Is direct access still supported?

@vsukhin
Copy link
Contributor

vsukhin commented Nov 11, 2024

hey, @thradec No, direct access feature was not touched, we just remove existed cli oauth support, because it was limited and it looked like no one used it. Do you think ingress definition is useful for you in our helm chart @rangoo94 @ypoplavs ?

@rangoo94
Copy link
Member Author

I don't think CLI ingress was used for it - from what I know, direct was using simply direct URL - often pointed to testkube-api-server service. I won't be next to the computer though today, so I'm not able to confirm.

@thradec
Copy link

thradec commented Nov 12, 2024

Hi, we were using direct access and call testkube api from outside the cluster, so the calls go through ingress which is exposing the api. Or should we migrate to ui-ingress now?

For context, this is the example how we are using it:

$ testkube --client direct --api-uri https://<cluster-hostname>/testkube/api run tw <test-workflow>

@rangoo94
Copy link
Member Author

@emil2k, @ypoplavs - could you recommend?

We've been earlier having Ingress created based on the cliIngress property. I'm not sure what was the original intent around that - I'm not sure if it's safe to expose it this way. It was not enabled by default though.

We can either:

  • add it back, and just delete oauth configuration from there
  • recommend adding Ingress resource separately

I think that it may be better to add Ingress separately, but I have limited knowledge about that.

@rangoo94
Copy link
Member Author

@emil2k, @ypoplavs - if you think that it's fine to keep cliIngress property, I have prepared helm-charts#962 PR that will add this functionality back 👍

@rangoo94
Copy link
Member Author

@thradec - I have added back cliIngress, as it seems to not cause any problems as it's disabled by default. The newest Helm Charts should work fine for you 👍

@thradec
Copy link

thradec commented Nov 12, 2024

Thanks a lot! ❤️

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.

3 participants