Skip to content
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

Move SSH configuration into its own structure #673

Merged
merged 7 commits into from
Apr 9, 2019

Conversation

martina-if
Copy link
Contributor

@martina-if martina-if commented Mar 28, 2019

Move SSH configuration into it's own struct in preparation for #386

  • Code compiles correctly (i.e make build)
  • Added tests that cover your change (if possible)
  • All unit tests passing (i.e. make test)
  • Manually tested both the config file and the cli parameters
  • Added/modified documentation as required (such as the README.md, and examples directory)

@martina-if martina-if changed the title Move SSH configuration into its own structure WIP: Move SSH configuration into its own structure Mar 28, 2019
errordeveloper
errordeveloper previously approved these changes Mar 28, 2019
Copy link
Contributor

@errordeveloper errordeveloper left a comment

Choose a reason for hiding this comment

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

Thanks @martina-if! Probably best get this in as-is and make the rest of changes for #386 in another PR.

@errordeveloper
Copy link
Contributor

errordeveloper commented Mar 28, 2019

closes #386

There're quite a few things in that issue, do you want to make it all happen in one PR? Up to you.

To be clear, I think you'll need to change those string fields to *string, and make a few changes in key loader function, so that it respects PublicKey and PublicKeyName (as currently it doesn't).

@martina-if martina-if force-pushed the move-ssh-to-struct branch 4 times, most recently from 4b937cc to 07e4afb Compare March 29, 2019 14:34
@martina-if martina-if changed the title WIP: Move SSH configuration into its own structure Move SSH configuration into its own structure Mar 29, 2019
@martina-if martina-if changed the title Move SSH configuration into its own structure WIP: Move SSH configuration into its own structure Mar 29, 2019
@martina-if martina-if changed the title WIP: Move SSH configuration into its own structure Move SSH configuration into its own structure Apr 2, 2019
pkg/eks/auth.go Outdated Show resolved Hide resolved
Copy link
Contributor

@errordeveloper errordeveloper left a comment

Choose a reason for hiding this comment

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

A few comments... I didn't read all of LoadSSHPublicKey just yet.

@martina-if martina-if force-pushed the move-ssh-to-struct branch 2 times, most recently from c62088c to 5a7df58 Compare April 8, 2019 11:55
Copy link
Contributor

@errordeveloper errordeveloper left a comment

Choose a reason for hiding this comment

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

🚢LGMT!

@phlukman
Copy link

hi @martina-if

I'm running version 0.1.29 and I'm configuring the yaml file as follows, according to the examples:

nodeGroups:

  • name: security-cluster-ng1-public
    instanceType: t2.2xlarge
    desiredCapacity: 1
    ssh:
    publicKeyName: security-es-cluster-key
  • name: security-cluster-ng2-private
    instanceType: t2.2xlarge
    desiredCapacity: 2
    privateNetworking: true
    ssh:
    publicKeyName: security-es-cluster-key

and I'm getting error:
[✖] loading config file "secure-cluster-config.yaml": error unmarshaling JSON: while decoding JSON: json: unknown field "ssh"

What am I missing here?

@martina-if
Copy link
Contributor Author

Hi @phlukman,

The new ssh schema hasn't been released yet. We will make a release with those changes tomorrow at the latest. In the meantime try using it with the old schema, like this:

nodeGroups:
  - name: security-cluster-ng1-public
    instanceType: t2.2xlarge
    desiredCapacity: 1
    sshPublicKeyPath: security-es-cluster-key

  - name: security-cluster-ng2-private
    instanceType: t2.2xlarge
    desiredCapacity: 2
    privateNetworking: true
    sshPublicKeyPath: security-es-cluster-key

@phlukman
Copy link

thanks for your quick reply @martina-if !

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