Fix nil linuxprofile #2621
Fix nil linuxprofile #2621
Conversation
|
UT? |
|
@zqingqing1 @weinong the new testdata file suggests nil pointer execution flow is still possible (see latest test run) |
|
yeh, working on this |
pkg/api/apiloader.go
Outdated
| SSHKeySize = 4096 | ||
| ) | ||
|
|
||
| func (a *Apiloader) createSSH(rg io.Reader) (publicKeyString string, err error) { |
There was a problem hiding this comment.
Is this function a duplicate of
acs-engine/pkg/acsengine/ssh.go
Line 46 in a0ad2d7
There was a problem hiding this comment.
I discussed with Weinong. I tried to move the code to Helpers folder, however, some structs in the ssh.go are in acsengine folder. If I import acsengine in this file, it will have the import cycle. moving has much work than we thought.
There was a problem hiding this comment.
I'm not seeing why we shouldn't do, as Cecile suggests:
- move the
SSHCreatortype struct + methods inacs-engine/pkg/acsengine/ssh.gotopkg/acgengine/helpers/helpers.go - convert existing usage patterns of
github.com/Azure/acs-engine.SSHCreatortogithub.aaakk.us.kg/Azure/acs-engine/helpers.SSHCreator - follow the above, new usage pattern in
pkg/api
There was a problem hiding this comment.
In the function createSaveSSH(although I don't use this function) in the ssh.go file, there is a struct called FileSaver and 2 functions privateKeyToPem(pki.go) SaveFile() are at acsengine folder. should we move the pki.go and other related file to helpers folder? Actually, we can leave that function there, and just move the CreateSSH() to helpers.go, But ssh_test.go actually called CreateSSH and privateKeyToPem functions, where should the ssh_test.go be?
There was a problem hiding this comment.
I would move any tests of moved code to the new package namespace. I want to keep things as simple as possible, we just don't want to have duplicate code.
This reverts commit 7272611.
* check if linuxprofile is nil
What this PR does / why we need it: Need to check if LinuxProfile is nil, otherwise raised nil pointer panic.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
If applicable:
Release note: