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

add support for --tls flag (#146) #360

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

r-mol
Copy link

@r-mol r-mol commented Apr 22, 2023

What was changed

  1. cli/app.go -> Added --tls flag to app.flags slice
  2. cli/factory.go -> Added parsing --tls flag with checking on error. And added the logic that when the flag is activated, the host is taken from the address/localhost
  3. cli/flags.go -> Added const of --tls flag name
  4. cli_curr/app.go -> Added --tls flag to app.flags slice
  5. cli_curr/factory.go -> Added parsing --tls flag with checking on error. And added the logic that when the flag is activated, the host is taken from the address/localhost

Why?

I am connecting to my server over a TLS connection and I had to duplicate my host from the --address flag to the --tls_server_name flag. This cluttered up the command, as well as misleading.

Checklist

  1. Closes #146

  2. How was this tested:

  • By internal tests, but with adding this flag
  • By connecting to local server with this flag
  1. Any docs updates needed? - NO

Related to same issue

temporalio/cli#210

* refactor: add new flag --tls, to create tls connection by host from the given address

* refactor: сhange parsing value of args

* refactor: сhange usage

* refactor: change the logic. (If we have server name, then it is not need to get tls flag)

* refactor: change parsing of TLS flag

* refactor: change parsing flag address

* refactor: rewrite comment

* refactor: rewrite comment
@CLAassistant
Copy link

CLAassistant commented Apr 22, 2023

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@r-mol r-mol changed the title Add TLS flag add support for --tls flag (#146) Apr 22, 2023
@r-mol
Copy link
Author

r-mol commented Jun 9, 2023

Hi! It's been a while since my request was approved for the merge, but I haven't heard any updates yet on when it will be merged. Is there anything else that needs to be done on my end, or something else that is slowing down the merge process? Let me know if there's anything I can do to help. Thanks!

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.

Toggle the TLS configuration based on the Frontend URL format
3 participants