Resolves: MTV-4378 | Add TLS certificate validation for Hyper-V provider#2231
Resolves: MTV-4378 | Add TLS certificate validation for Hyper-V provider#2231avivtur merged 1 commit intokubev2v:mainfrom
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2231 +/- ##
===========================================
- Coverage 36.81% 14.58% -22.23%
===========================================
Files 158 1127 +969
Lines 2548 20187 +17639
Branches 599 3773 +3174
===========================================
+ Hits 938 2944 +2006
- Misses 1428 17233 +15805
+ Partials 182 10 -172 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Hazanel
force-pushed
the
hyperv-tls-certificate
branch
5 times, most recently
from
4e1f8d0 to
75f2cdc
Compare
avivtur
left a comment
avivtur
left a comment
There was a problem hiding this comment.
please look in file src/overview/tabs/Overview/card/Welcome/WelcomeCard.tsx
in the we render provider cards to the screen of the overview page, please add a ProviderCard for hyper-v
| {showPreserveIPWarningsConditions && ( | ||
| <StackItem> | ||
| <PlanWarningAlerts conditions={preserveIPWarningsConditions} /> | ||
| </StackItem> |
There was a problem hiding this comment.
why adding warning alerts? who asked for this? this could be a significant UX change that should confirmed before added please remove
There was a problem hiding this comment.
remove file, please refer to comment above
|
|
||
| type CertificateUploadProps = FileUploadProps & { | ||
| url?: string; | ||
| onFetchedCertificate?: (cert: string) => void; |
There was a problem hiding this comment.
why you need this? the onChange should function in the same way, this file shouldn't really change
| launcher<FetchCertificateModalProps>(FetchCertificateModal, { | ||
| existingCert: (value as string) ?? '', | ||
| handleSave: (saved) => { | ||
| onTextChange?.(syntheticEvent, saved); | ||
| }, | ||
| url: url ?? '', | ||
| }); |
There was a problem hiding this comment.
this launcher method is an API provided from console we are using to render modal dialogs, in general if you're adding a modal dialog you should use this
| )} | ||
| </FileUpload> | ||
|
|
||
| {isModalOpen && ( |
There was a problem hiding this comment.
what is the reason to use inline render for the modal rather than using the launcher API?
There was a problem hiding this comment.
revert changes in this file, onFetchedCertificate is not need here
| if (providerType === PROVIDER_TYPES.hyperv) { | ||
| // Format as https URL for certificate fetching (WinRM over HTTPS uses port 5986) | ||
| const host = (hypervHost as string)?.trim(); | ||
| return host ? `https://${host}:5986` : ''; |
There was a problem hiding this comment.
replace 5986 with const something like
const WIN_RM_PORT = 5986;
There was a problem hiding this comment.
file not needed please remove and use FetchCertificateModal that you deleted
Hazanel
force-pushed
the
hyperv-tls-certificate
branch
3 times, most recently
from
320cfdf to
22145ea
Compare
Hazanel
force-pushed
the
hyperv-tls-certificate
branch
from
22145ea to
53f9ef8
Compare
|
LGTM |
- Add CertificateValidationField to Hyper-V provider creation form - Include cacert and insecureSkipVerify in Hyper-V provider secret - Display certificate fields in provider credentials view - Support certificate configuration in edit provider flow - Add WIN_RM_PORT constant for WinRM HTTPS port - Add Hyper-V ProviderCard to Overview Welcome page Ref: https://issues.redhat.com/browse/MTV-4378 Signed-off-by: Elad Hazan <ehazan@redhat.com>
Hazanel
force-pushed
the
hyperv-tls-certificate
branch
from
5f8a0e4 to
bf68b07
Compare
|
3 participants
Ref: https://issues.redhat.com/browse/MTV-4378