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

4830 - Fix show / Hide gitrepo create fields based on isTLS. #4893

Merged

Conversation

Shavindra
Copy link
Contributor

@Shavindra Shavindra commented Jan 10, 2022

Summary

Issue #4830 - Form elements incorrectly hidden in Continuous Delivery Git Repo create/edit form for http:// repository URL.

Steps to test

  1. Navigate to Continuous delivery > Git repos.
  2. Click on create repo.
  3. Paste a https:// github URL e.g. https://github.com/rancher/fleet-examples.git
  4. Remove the s from https e.g. http://github.com/rancher/fleet-examples.git
  5. It should hide TLS Certificate Verification field when its http. Other fields should remain.
Screen.Recording.2022-01-20.at.14.24.19.mov

@Shavindra Shavindra marked this pull request as ready for review January 20, 2022 13:15
Copy link
Member

@richard-cox richard-cox left a comment

Choose a reason for hiding this comment

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

The core change looks good and resolves the issue as stated, however it exposes another issue that will probably cause #4830 to fail QA so should be resolved at the same time.

When a non-tls url is supplied the two settings associated with tls aren't changed. Think this is fine for insecureSkipTLSVerify (guess this is ignored in the non-tls case) but it looks like we do custom stuff with caBundle on reload. To be safe it's probably best to clear both before the git repo is saved. This can be done in the same component using a registerBeforeHook.

@richard-cox richard-cox self-requested a review February 28, 2022 16:30
@nwmac
Copy link
Member

nwmac commented Mar 1, 2022

@Shavindra You can go ahead and merge this

@nwmac nwmac added this to the 2.6.4 milestone Mar 1, 2022
@Shavindra Shavindra merged commit 6bc736e into rancher:master Mar 1, 2022
@Shavindra Shavindra deleted the 4830-incorrectly-hidden-istls-fields branch March 1, 2022 15:56
@nwmac nwmac modified the milestones: 2.6.4, v2.6.4 Mar 1, 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.

None yet

3 participants