-
Notifications
You must be signed in to change notification settings - Fork 22
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
OCM-13040 | test: Bastion proxy support username and password #81
base: main
Are you sure you want to change the base?
Conversation
b976ed6
to
0eef957
Compare
pkg/test/vpc_client/bastion.go
Outdated
@@ -94,8 +98,8 @@ func (vpc *VPC) LaunchBastion(imageID string, zone string, userData string, keyp | |||
return inst, nil | |||
} | |||
|
|||
func (vpc *VPC) PrepareBastionProxy(zone string, cidrBlock string, keypairName string, | |||
privateKeyPath string) (*types.Instance, error) { | |||
func (vpc *VPC) PrepareBastionProxy(zone string, keypairName string, privateKeyPath string) (*types.Instance, string, |
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.
Can you please describe the meaning of return values in comment part?
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.
Add description of the return value.
pkg/test/vpc_client/bastion.go
Outdated
} | ||
|
||
username := utils.RandomLabel(5) | ||
password := utils.RandomLabel(5) |
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.
Please make the password more complicated.
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.
make the password length to 10.
pkg/test/vpc_client/bastion.go
Outdated
|
||
userData := fmt.Sprintf(`#!/bin/bash | ||
userData := `#!/bin/bash |
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.
Let's make the userData generation to another function? It's too big.
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.
Fixed.Use generateShellCommand() func to generate userData.
pkg/test/vpc_client/bastion.go
Outdated
return nil, "", "", err | ||
} | ||
|
||
localFilePath := "./tmp/passwords" |
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 is this? Can you confirm this path can be written in all kinds of containers? Better to mktempdir here.
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.
Use Ying's run ssh command to set password for bastion proxy, local file store password is not needed. This part has been removed.
pkg/test/vpc_client/bastion.go
Outdated
} | ||
log.LogInfo("Found existing bastion: %s", *insts[0].InstanceId) | ||
return &insts[0], nil | ||
return &insts[0], "", "", nil |
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.
Let's return the proxy url here rather than instance.
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.
Updated according to comments. Proxy url will be returned currently.
pkg/test/vpc_client/bastion.go
Outdated
return string(hashedPassword), nil | ||
} | ||
|
||
func writePasswordToFile(username, hashedPassword, filePath string) error { |
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.
Is there a common func defined for write file?
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.
Use Ying's run ssh command to set password for bastion proxy, local file store password is not needed. This part has been removed.
pkg/test/vpc_client/bastion.go
Outdated
} | ||
|
||
func loadPrivateKey(privateKeyPath, keypairName string) (ssh.Signer, error) { | ||
privateKeyName := fmt.Sprintf("%s/%s-%s", privateKeyPath, keypairName, "keyPair.pem") |
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.
User path.Join rather the /
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.
Updated according to comment.
pkg/test/vpc_client/bastion.go
Outdated
return signer, nil | ||
} | ||
|
||
func uploadFileToBastion(host, port, username, privateKeyPath, keypairName, localFilePath, remoteFilePath string) error { |
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.
There is already a function defined to run ssh command please check with Ying.
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.
Updated according to comment.
0eef957
to
91c5622
Compare
No description provided.