[GitHub] cloudstack pull request: CLOUDSTACK-5822: keep user-added sshkeys ...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1044#issuecomment-175677195
  
LGTM
Merging based on 2+LGTMs, test results shared in comments


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-5822: keep user-added sshkeys ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/cloudstack/pull/1044


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-5822: keep user-added sshkeys ...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1044#issuecomment-162476357
  
regression tests executed:

[1044.network.results.txt](https://github.com/apache/cloudstack/files/53778/1044.network.results.txt)

[1044.vpc.results.txt](https://github.com/apache/cloudstack/files/53779/1044.vpc.results.txt)
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-5822: keep user-added sshkeys ...

[NuxRo]

Github user NuxRo commented on the pull request:

https://github.com/apache/cloudstack/pull/1044#issuecomment-155735899
  
On a second thought - and something worth pondering on - this could have 
some security implications.

Imagine you have a private cloud, a developer/employee leaves and you want 
to remove his key from the instances because "security". People used to the old 
behaviour might think they're safe when they are in fact not.
Thoughts?

Now, multi-key support, that'd be terrific. :-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-5822: keep user-added sshkeys ...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1044#issuecomment-155737852
  
@NuxRo valid point but isn't this unexpected behavior instead of expected? 
The key was not added by the UI (or API) but will be removed by it. If we need 
this a seperate API, resetAllSshKeysInVm should be made.

An angry employee having keys on a vm (out of band) is a real and present 
danger, indeed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-5822: keep user-added sshkeys ...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/1044#issuecomment-155734540
  
@ustcweizhou 

How did you test it?

I think we should stick to a LGTM being given only if tests have been done 
and steps, on how to test, have been made clear. 

Cheers,
Wilder


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-5822: keep user-added sshkeys ...

[NuxRo]

Github user NuxRo commented on the pull request:

https://github.com/apache/cloudstack/pull/1044#issuecomment-155740283
  
Good point as well with the "unexpected". I definitely see where Wei is 
coming from, but I think it could be misleading. Perhaps a better way to do 
this is mark the ACS key and only reset that one. i.e.
When we add the key append a "# added by Cloudstack" and when we issue a 
reset, just delete that one.

Am I overcomplicating this? I might be, especially as these scripts are 
being slowly phased out.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-5822: keep user-added sshkeys ...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1044#issuecomment-155748424
  
@NuxRo @ustcweizhou is using "cloudst...@apache.org$" for this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-5822: keep user-added sshkeys ...

[NuxRo]

Github user NuxRo commented on the pull request:

https://github.com/apache/cloudstack/pull/1044#issuecomment-155751828
  
Ah, right, checking the code is important. :-D
I'll go back to my corner.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-5822: keep user-added sshkeys ...

[wido]

Github user wido commented on the pull request:

https://github.com/apache/cloudstack/pull/1044#issuecomment-155003768
  
Looks good, sane commit.

I agree with @NuxRo that cloud-init is imho the way forward. We probably 
want to ditch these legacy scripts at some point.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-5822: keep user-added sshkeys ...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1044#issuecomment-154691168
  
makes sense and looks good. @ustcweizhou I assume you have tested this in 
production already, no?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-5822: keep user-added sshkeys ...

[NuxRo]

Github user NuxRo commented on the pull request:

https://github.com/apache/cloudstack/pull/1044#issuecomment-154707648
  
Makes sense and looks good, but I am surprised you are still using these 
scripts instead of cloud-init.
On the same note, worth having a look at how cloud-init behaves, I'll give 
it a try when I get some time.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-5822: keep user-added sshkeys ...

[ustcweizhou]

GitHub user ustcweizhou opened a pull request:

https://github.com/apache/cloudstack/pull/1044

CLOUDSTACK-5822: keep user-added sshkeys in authorized_keys

For now, if we add the ssh key inside the vm (not on cloudstack UI), the 
sshkey will be removed if we reset the sshkey on cloudstack UI.

After this commit, the sshkey (added by cloudstack) will end with 
cloudst...@apache.org.
We will only control the sshkeys with cloudst...@apache.org.

This will be used for multiple sshkey support for vm in the future.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ustcweizhou/cloudstack keep-sshkey

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1044.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1044


commit 64ef4fa958878ceedfc8009804898f439cdeaf0f
Author: Wei Zhou 
Date:   2015-11-06T13:28:14Z

CLOUDSTACK-5822: keep user-added sshkeys in authorized_keys




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---