Skip to content

Conversation

@guusvw
Copy link
Contributor

@guusvw guusvw commented Jan 6, 2018

No description provided.

@mrIncompetent
Copy link
Contributor

@guusvw rebase please

Has this been tested?

@guusvw
Copy link
Contributor Author

guusvw commented Jan 10, 2018

(Rebase )Will do it and I will also write an test for EnsureSSHKeypairSecret to ensure the same behaviour as before.

@mrIncompetent
Copy link
Contributor

We indeed are missing some tests

@guusvw guusvw force-pushed the feature/some-refactoring branch from b021721 to 575f3a3 Compare January 10, 2018 08:46
AddToScheme(scheme)
}

// AddToScheme adds all types of this clientset into the given scheme. This allows composition
Copy link
Contributor

Choose a reason for hiding this comment

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

everything under pkg/client is generated code

@guusvw guusvw force-pushed the feature/some-refactoring branch 2 times, most recently from 876c43c to 2cdd19c Compare January 10, 2018 11:24
@guusvw guusvw changed the title WIP: Feature/some refactoring Feature/some refactoring Jan 10, 2018
func EnsureSSHKeypairSecret(client kubernetes.Interface) (*rsa.PrivateKey, error) {
if client == nil {
return nil, fmt.Errorf("got an nil k8s client")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really wan to do a nil check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes ;) make the code more stable

return nil, fmt.Errorf("key data not found in secret '%s/%s' (secret.data['%s']). remove it and a new one will be created", secret.Namespace, secret.Name, privateKeyDataIndex)
}
if len(b) == 0 {
return nil, fmt.Errorf("key data not found in secret '%s/%s' (secret.data['%s']). remove it and a new one will be created", secret.Namespace, secret.Name, privateKeyDataIndex)
Copy link
Contributor

Choose a reason for hiding this comment

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

i would rather state something like invalid key data found

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@guusvw guusvw force-pushed the feature/some-refactoring branch from c953e3d to d8c491c Compare January 12, 2018 16:46
@mrIncompetent mrIncompetent merged commit de8bb7e into master Jan 12, 2018
@mrIncompetent mrIncompetent deleted the feature/some-refactoring branch January 12, 2018 16:58
irozzo-1A referenced this pull request in irozzo-1A/machine-controller Mar 10, 2020
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.

3 participants