-
Notifications
You must be signed in to change notification settings - Fork 19
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
Implement a new kubectl transport #60
Merged
greg-1-anderson
merged 5 commits into
consolidation:main
from
refstudycentre:dev-kubectl-transport
Feb 19, 2022
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
ddfcf81
Implement a new kubectl transport for accessing Drupal instances runn…
rudolfbyker cc45af4
Add tests for kubectl transport.
rudolfbyker 0bc4acd
Update CHANGELOG.md
rudolfbyker 8d36fa9
Do not escape arguments after "--"
rudolfbyker 6cd16eb
PHPCS Beautify
rudolfbyker File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
<?php | ||
|
||
namespace Consolidation\SiteProcess\Factory; | ||
|
||
use Consolidation\SiteAlias\SiteAliasInterface; | ||
use Consolidation\SiteProcess\Transport\KubectlTransport; | ||
|
||
/** | ||
* KubectlTransportFactory will create an KubectlTransport for applicable site aliases. | ||
*/ | ||
class KubectlTransportFactory implements TransportFactoryInterface | ||
{ | ||
/** | ||
* @inheritdoc | ||
*/ | ||
public function check(SiteAliasInterface $siteAlias) | ||
{ | ||
return $siteAlias->has('kubectl'); | ||
} | ||
|
||
/** | ||
* @inheritdoc | ||
*/ | ||
public function create(SiteAliasInterface $siteAlias) | ||
{ | ||
return new KubectlTransport($siteAlias); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
<?php | ||
|
||
namespace Consolidation\SiteProcess\Transport; | ||
|
||
use Consolidation\SiteProcess\SiteProcess; | ||
use Consolidation\SiteAlias\SiteAliasInterface; | ||
use Consolidation\SiteProcess\Util\Shell; | ||
|
||
/** | ||
* KubectlTransport knows how to wrap a command such that it runs in a container | ||
* on Kubernetes via kubectl. | ||
*/ | ||
class KubectlTransport implements TransportInterface | ||
{ | ||
/** @var bool */ | ||
protected $tty; | ||
|
||
/** @var \Consolidation\SiteAlias\SiteAliasInterface */ | ||
protected $siteAlias; | ||
|
||
public function __construct(SiteAliasInterface $siteAlias) | ||
{ | ||
$this->siteAlias = $siteAlias; | ||
} | ||
|
||
/** | ||
* @inheritdoc | ||
*/ | ||
public function configure(SiteProcess $process) | ||
{ | ||
$this->tty = $process->isTty(); | ||
} | ||
|
||
/** | ||
* inheritdoc | ||
*/ | ||
public function wrap($args) | ||
{ | ||
# TODO: How/where do we complain if a required argument is not available? | ||
$namespace = $this->siteAlias->get('kubectl.namespace'); | ||
$tty = $this->tty && $this->siteAlias->get('kubectl.tty', false) ? "true" : "false"; | ||
$interactive = $this->tty && $this->siteAlias->get('kubectl.interactive', false) ? "true" : "false"; | ||
$resource = $this->siteAlias->get('kubectl.resource'); | ||
$container = $this->siteAlias->get('kubectl.container'); | ||
|
||
$transport = [ | ||
'kubectl', | ||
"--namespace=$namespace", | ||
'exec', | ||
"--tty=$tty", | ||
"--stdin=$interactive", | ||
$resource, | ||
]; | ||
if ($container) { | ||
$transport[] = "--container=$container"; | ||
} | ||
$transport[] = "--"; | ||
|
||
return array_merge($transport, $args); | ||
} | ||
|
||
/** | ||
* @inheritdoc | ||
*/ | ||
public function addChdir($cd_remote, $args) | ||
{ | ||
return array_merge( | ||
[ | ||
'cd', | ||
$cd_remote, | ||
Shell::op('&&'), | ||
], | ||
$args | ||
); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
<?php | ||
|
||
namespace Consolidation\SiteProcess; | ||
|
||
use Consolidation\SiteProcess\Transport\KubectlTransport; | ||
use PHPUnit\Framework\TestCase; | ||
use Consolidation\SiteAlias\SiteAlias; | ||
|
||
class KubectlTransportTest extends TestCase | ||
{ | ||
/** | ||
* Data provider for testWrap. | ||
*/ | ||
public function wrapTestValues() | ||
{ | ||
return [ | ||
// Everything explicit. | ||
[ | ||
'kubectl --namespace=vv exec --tty=false --stdin=false deploy/drupal --container=drupal -- ls', | ||
['ls'], | ||
[ | ||
'kubectl' => [ | ||
'tty' => false, | ||
'interactive' => false, | ||
'namespace' => 'vv', | ||
'resource' => 'deploy/drupal', | ||
'container' => 'drupal', | ||
] | ||
], | ||
], | ||
|
||
// Minimal. Kubectl will pick a container. | ||
[ | ||
'kubectl --namespace=vv exec --tty=false --stdin=false deploy/drupal -- ls', | ||
['ls'], | ||
[ | ||
'kubectl' => [ | ||
'namespace' => 'vv', | ||
'resource' => 'deploy/drupal', | ||
] | ||
], | ||
], | ||
|
||
// Don't escape arguments after "--" | ||
[ | ||
'kubectl --namespace=vv exec --tty=false --stdin=false deploy/drupal -- asdf "double" \'single\'', | ||
['asdf', '"double"', "'single'"], | ||
[ | ||
'kubectl' => [ | ||
'namespace' => 'vv', | ||
'resource' => 'deploy/drupal', | ||
] | ||
], | ||
], | ||
]; | ||
} | ||
|
||
/** | ||
* @dataProvider wrapTestValues | ||
*/ | ||
public function testWrap($expected, $args, $siteAliasData) | ||
{ | ||
$siteAlias = new SiteAlias($siteAliasData, '@alias.dev'); | ||
$dockerTransport = new KubectlTransport($siteAlias); | ||
$actual = $dockerTransport->wrap($args); | ||
$this->assertEquals($expected, implode(' ', $actual)); | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@greg-1-anderson How/where do we complain if a required argument in is not available in the site alias? Is there a "standard" way (e.g. a specific custom exception that should be thrown) in this repo?
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.
Throwing an exception would be best; however, at the moment, this library does not provide any custom exception types. You could provide one, if you wanted, or maybe just use InvalidArgumentException. Maybe that's just similar enough to be confusing, though. Recommendations welcome.
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.
I would leave the TODO there, merge it, and make a new issue to address the need for custom exceptions. It needs to be designed well.
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.
#62