-
Notifications
You must be signed in to change notification settings - Fork 6
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 verifyCertificates option and 'Run zwe init certificates' button to Certificate Stage #79
Add verifyCertificates option and 'Run zwe init certificates' button to Certificate Stage #79
Conversation
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
…efactor and implement form change handler Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
…unning zwe init certificates Signed-off-by: Timothy Gerstel <[email protected]>
…ly and remove console logs Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
…recate ConfigurationStore somehow Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
…ertificateStage Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
src/actions/InstallationHandler.ts
Outdated
return ProgressStore.set('certificate.uploadYaml', false);; | ||
} | ||
ProgressStore.set('certificate.uploadYaml', uploadYaml.status); | ||
const script = `cd ${installDir}/runtime/bin;\n./zwe init certificate -c ${installDir}/zowe.yaml`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zwe init has many optional arguments. how can we let the user choose?
https://docs.zowe.org/stable/appendix/zwe_server_command_reference/zwe/init/zwe-init-certificate
--update-config
is a good arg (probably we should default to it!) but it requires us to pull back the yaml when done, and consume it to get the changes.
--allow-overwrite
also seems like a good arg, because without it you could just fail if the operation were done twice for 2 different installs.
--security-dry-run
and similar dry-run stuff will be needed in a future ticket, so that we can show a preview before we do a thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree but I think this is out of scope for the ticket
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm sure you
- want
--update-config
and--allow-overwrite
as a default, unless you like failures and manual labor (bugs me that these arent defaults in zwe itself) - want to make a function that takes these as arguments so that in another ticket, you can make a UI that varies the arguments.
const setupSchema = schema ? schema.properties.zowe.properties.setup.properties.certificate : ""; | ||
const verifyCertsSchema = schema ? {"type": "object", "properties": {"verifyCertificates": schema.properties.zowe.properties.verifyCertificates}} : ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this mean? why would our schema be empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this because this is how @sakshibobade21 implemented it after implementing JsonForms:
https://github.com/zowe/zen/blob/v2.x/staging/src/renderer/components/stages/Security.tsx#L26
https://github.com/zowe/zen/blob/v2.x/staging/src/renderer/components/stages/installation/Installation.tsx#L33
Figured maybe she knew something we didn't
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
console.log("uploading yaml..."); | ||
const uploadYaml = await this.uploadYaml(connectionArgs, installDir); | ||
if(!uploadYaml.status){ | ||
return ProgressStore.set('certificate.uploadYaml', false);; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double semi-colon
const [isFormValid, setIsFormValid] = useState(false); | ||
const [formError, setFormError] = useState(''); | ||
|
||
const section = 'certificate'; | ||
const initConfig: any = getConfiguration(section); | ||
|
||
const verifyCertsConfig: any = (getZoweConfig() as any).zowe.verifyCertificates; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gives me a run-time error at certificates step
[14660:1102/075437.976:INFO:CONSOLE(46)]
"Uncaught TypeError: Cannot read properties of undefined (reading 'zowe')", source: webpack-internal:///./src/renderer/components/stages/Certificates.tsx (46)
[14660:1102/075437.987:INFO:CONSOLE(46)] "Uncaught TypeError: Cannot read properties of undefined (reading 'zowe')", source: webpack-internal:///./src/renderer/components/stages/Certificates.tsx (46)
[14660:1102/075437.993:INFO:CONSOLE(18687)] "The above error occurred in the component:
at Certificates (webpack-internal:///./src/renderer/components/stages/Certificates.tsx:30:49)
at div
at div
at eval (webpack-internal:///./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js:57:66)
at Box (webpack-internal:///./node_modules/@mui/system/esm/createBox.js:34:72)
at HorizontalLinearStepper (webpack-internal:///./src/renderer/components/common/Stepper.tsx:49:13)
at div
at Wizard (webpack-internal:///./src/renderer/components/configuration-wizard/Wizard.tsx:37:133)
at RenderedRoute (webpack-internal:///./node_modules/react-router/dist/index.js:715:5)
at Outlet (webpack-internal:///./node_modules/react-router/dist/index.js:1118:26)
at Header (webpack-internal:///./src/renderer/components/Header.tsx:47:60)
at RenderedRoute (webpack-internal:///./node_modules/react-router/dist/index.js:715:5)
at Routes (webpack-internal:///./node_modules/react-router/dist/index.js:1209:5)
at Router (webpack-internal:///./node_modules/react-router/dist/index.js:1141:15)
at HashRouter (webpack-internal:///./node_modules/react-router-dom/dist/index.js:372:5)
at InnerThemeProvider (webpack-internal:///./node_modules/@mui/system/esm/ThemeProvider/ThemeProvider.js:23:70)
at ThemeProvider (webpack-internal:///./node_modules/@mui/private-theming/ThemeProvider/ThemeProvider.js:45:5)
at ThemeProvider (webpack-internal:///./node_modules/@mui/system/esm/ThemeProvider/ThemeProvider.js:42:5)
at Provider (webpack-internal:///./node_modules/react-redux/es/components/Provider.js:16:3)
at App
Consider adding an error boundary to your tree to customize error handling behavior.
Visit https://reactjs.org/link/error-boundaries to learn more about error boundaries.", source: webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js (18687)
[14660:1102/075437.994:INFO:CONSOLE(12056)] "Uncaught TypeError: Cannot read properties of undefined (reading 'zowe')", source: webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js (12056)
`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is achieved by running through the steps naturally,
and at Step 6 it's run-time TS error. Whatever the cause of the error, it's safer to do something like:
const verifyCertsConfig: any = (getZoweConfig()?.zowe)?.verifyCertificates;
Lmk if you need help replicating the run-time error!
Signed-off-by: Timothy Gerstel <[email protected]>
…ertificateStage Signed-off-by: Timothy Gerstel <[email protected]>
…to enhancement/certificateStage Signed-off-by: Timothy Gerstel <[email protected]>
…ertificateStage Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
…te stage Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
Signed-off-by: Timothy Gerstel <[email protected]>
@@ -37,6 +37,28 @@ export const setConfiguration = (section: string, data: any, setZconfig?: boolea | |||
} | |||
} | |||
|
|||
export const setTopLevelZConfig = (section: string, data: any) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this now code duplicate with ?
setConfiguration = (section: string, data: any, setZconfig?: TRUE) => {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This actually already exists on staging so that needs to be removed from this PR anyway:
https://github.com/zowe/zen/blob/v2.x/staging/src/services/ConfigService.ts#L64
You are correct that there was 100% a better way to implement the needed functionality from the original function, but this one updates any level of the yaml instead of just zowe.setup. I should have just made the original update any section of the yaml, then updated the existing references I suppose. This must have been a bad time saving hack i though was appropriate at the time. Should be refactored.
Proposed changes
This pull request implements the following changes:
This PR depends upon the following PRs:
Type of change
Please delete options that are not relevant.
PR Checklist
Please delete options that are not relevant.
Testing
Further comments