Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Disable SSH probing on GCE #7348
Conversation
|
!!test!! |
| func (facade *Facade) AllAddresses(args params.Entities) (params.SSHAddressesResults, error) { | ||
| if err := facade.checkIsModelAdmin(); err != nil { | ||
| return params.SSHAddressesResults{}, errors.Trace(err) | ||
| } | ||
| + env, err := environs.GetEnviron(facade.configGetter, environs.New) |
jameinel
May 17, 2017
Owner
Layering wise, I know we've done this elsewhere, but it really feels unclean. I'm not sure that I have better answers at this moment.
It feels a bit like we should be recording 'addresses to use for SSH for this machine' in state, as part of the addressupdater worker, rather than having yet-another worker that needs to talk to the underlying provider.
maybe its the sanest first pass at this
| + environ, ok := environs.SupportsNetworking(env) |
jameinel
May 17, 2017
Owner
I'm not sure this is valid. I have the feeling places like 'Joyent' don't implement the Networking pieces of the Environ interface, but they still have IP addresses for instances, and you still want to SSH to them.
| @@ -172,7 +195,7 @@ func (facade *Facade) Proxy() (params.SSHProxyResult, error) { | ||
| if err := facade.checkIsModelAdmin(); err != nil { | ||
| return params.SSHProxyResult{}, errors.Trace(err) | ||
| } | ||
| - config, err := facade.backend.ModelConfig() | ||
| + config, err := facade.configGetter.ModelConfig() |
jameinel
May 17, 2017
Owner
why did this have to change?
It seems like we might have just added the extra methods that ConfigGetter needs as part of 'Backend', as then Backend can trivially implement EnvironConfigGetter rather than needing to have 2 separate interfaces that are really the same object.
| c.Assert(err, jc.ErrorIsNil) | ||
| s.facade = facade | ||
| } | ||
| func (s *facadeSuite) TestMachineAuthNotAllowed(c *gc.C) { | ||
| s.authorizer.Tag = names.NewMachineTag("0") | ||
| - _, err := sshclient.New(s.backend, nil, s.authorizer) | ||
| + _, err := sshclient.New(s.backend, s.backend, nil, s.authorizer) |
|
!!test!! |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
wupeka commentedMay 16, 2017
•
Edited 1 time
-
jameinel
May 16, 2017
Description of change
Due to SSH probing sshguard was triggered on GCE instances. This change causes SSH on GCE to only use one public address, without probing.
QA steps
Bootstrap on GCE, try juju ssh -m controller 0 a few times, it should work every time (sshguard should not be triggered)
Documentation changes
juju ssh is not probing all addresses on GCE, only the prefered public one.
Bug reference
https://bugs.launchpad.net/juju/+bug/1669501