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

Update K8s.Conn.from_file to support fully qualified domain names (FQDN) #164

Merged
merged 1 commit into from
Dec 13, 2022

Conversation

thephw
Copy link
Contributor

@thephw thephw commented May 19, 2022

Changelog

- Ref: coryodaniel#144
- I missed a codepath in the original PR that makes this not work with a
  call from K8s.Conn.from_file when using a FQDN for the server url.
@thephw
Copy link
Contributor Author

thephw commented May 19, 2022

@mruoss I feel like there should probably be an additional test for this as well, but we'd need to add a cluster fixture with a proper domain and TLS certificate. I am not certain that's worth the effort. Thoughts?

@mruoss
Copy link
Collaborator

mruoss commented May 23, 2022

All integration tests could connect to the cluster via FQDN, no?

@mruoss
Copy link
Collaborator

mruoss commented May 23, 2022

Maybe you can pull branch fqdn into your branch and see if you can write an integration test with this?

@thephw
Copy link
Contributor Author

thephw commented May 25, 2022

Ooooo that's clever, yeah let me give that a go tomorrow 👍🏻

@mruoss
Copy link
Collaborator

mruoss commented May 26, 2022

Changelog

Can you also update changelog.md?

@thephw
Copy link
Contributor Author

thephw commented May 29, 2022

Yes I can add that as well. I did not quite get this finished today and I am about to be out of town until 6/20. Hoping to snag a few minutes to wrap up, but otherwise I will pickup when I get back.

@mruoss
Copy link
Collaborator

mruoss commented Oct 3, 2022

Any chance you could wrap this up anytime soon?

@mruoss mruoss merged commit d6a4b8d into coryodaniel:develop Dec 13, 2022
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.

2 participants