-
Notifications
You must be signed in to change notification settings - Fork 402
DNM - connection/aw_sss - Use Port forwarding session for file transfer instead of S3 bucket #2265
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
base: main
Are you sure you want to change the base?
DNM - connection/aw_sss - Use Port forwarding session for file transfer instead of S3 bucket #2265
Conversation
fb41f29
to
08b355f
Compare
Docs Build 📝Thank you for contribution!✨ The docsite for this PR is available for download as an artifact from this run: You can compare to the docs for the File changes:
Click to see the diff comparison.NOTE: only file modifications are shown here. New and deleted files are excluded. diff --git a/home/runner/work/community.aws/community.aws/docsbuild/base/collections/community/aws/aws_ssm_connection.html b/home/runner/work/community.aws/community.aws/docsbuild/head/collections/community/aws/aws_ssm_connection.html
index b62a732..94ffd9e 100644
--- a/home/runner/work/community.aws/community.aws/docsbuild/base/collections/community/aws/aws_ssm_connection.html
+++ b/home/runner/work/community.aws/community.aws/docsbuild/head/collections/community/aws/aws_ssm_connection.html
@@ -245,6 +245,22 @@ see <a class="reference internal" href="#ansible-collections-community-aws-aws-s
</div></td>
</tr>
<tr class="row-odd"><td><div class="ansible-option-cell">
+<div class="ansibleOptionAnchor" id="parameter-host_port_number"></div><p class="ansible-option-title" id="ansible-collections-community-aws-aws-ssm-connection-parameter-host-port-number"><strong>host_port_number</strong></p>
+<a class="ansibleOptionLink" href="#parameter-host_port_number" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">integer</span></p>
+<p><em class="ansible-option-versionadded">added in community.aws 10.0.0</em></p>
+</div></td>
+<td><div class="ansible-option-cell"><p>The Port number of the server on the instance when using Port Forwarding Using AWS System Manager Session Manager to transfer files from/to local host to/from remote host.</p>
+<p>The port <code class="ansible-value docutils literal notranslate"><span class="pre">80</span></code> is used if not provided.</p>
+<p>The <code class="docutils literal notranslate"><span class="pre">nc</span></code> command should be installed in the remote host to use this option.</p>
+<p>This is not supported for Windows hosts for now.</p>
+<p class="ansible-option-line"><strong class="ansible-option-default-bold">Default:</strong> <code class="ansible-option-default docutils literal notranslate"><span class="pre">80</span></code></p>
+<p class="ansible-option-line"><strong class="ansible-option-configuration">Configuration:</strong></p>
+<ul class="simple">
+<li><p>Variable: ansible_aws_ssm_host_port_number</p></li>
+</ul>
+</div></td>
+</tr>
+<tr class="row-even"><td><div class="ansible-option-cell">
<div class="ansibleOptionAnchor" id="parameter-instance_id"></div><p class="ansible-option-title" id="ansible-collections-community-aws-aws-ssm-connection-parameter-instance-id"><strong>instance_id</strong></p>
<a class="ansibleOptionLink" href="#parameter-instance_id" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">string</span></p>
</div></td>
@@ -255,6 +271,21 @@ see <a class="reference internal" href="#ansible-collections-community-aws-aws-s
</ul>
</div></td>
</tr>
+<tr class="row-odd"><td><div class="ansible-option-cell">
+<div class="ansibleOptionAnchor" id="parameter-local_port_number"></div><p class="ansible-option-title" id="ansible-collections-community-aws-aws-ssm-connection-parameter-local-port-number"><strong>local_port_number</strong></p>
+<a class="ansibleOptionLink" href="#parameter-local_port_number" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">integer</span></p>
+<p><em class="ansible-option-versionadded">added in community.aws 10.0.0</em></p>
+</div></td>
+<td><div class="ansible-option-cell"><p>Port number on local machine to forward traffic to when using Port Forwarding Using AWS System Manager Session Manager to transfer files from/to local host to/from remote host.</p>
+<p>An open port is chosen at run-time if not provided.</p>
+<p>The <code class="docutils literal notranslate"><span class="pre">nc</span></code> command should be installed in the remote host to use this option.</p>
+<p>This is not supported for Windows hosts for now.</p>
+<p class="ansible-option-line"><strong class="ansible-option-configuration">Configuration:</strong></p>
+<ul class="simple">
+<li><p>Variable: ansible_aws_ssm_local_port_number</p></li>
+</ul>
+</div></td>
+</tr>
<tr class="row-even"><td><div class="ansible-option-cell">
<div class="ansibleOptionAnchor" id="parameter-plugin"></div><p class="ansible-option-title" id="ansible-collections-community-aws-aws-ssm-connection-parameter-plugin"><strong>plugin</strong></p>
<a class="ansibleOptionLink" href="#parameter-plugin" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">string</span></p>
|
Build failed. ✔️ ansible-galaxy-importer SUCCESS in 3m 18s (non-voting) |
Build failed. ❌ ansible-galaxy-importer FAILURE in 5m 08s (non-voting) |
Build succeeded. ❌ ansible-galaxy-importer FAILURE in 4m 36s (non-voting) |
ce819ea
to
e546ce9
Compare
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 5m 15s (non-voting) |
a7c7b4f
to
5c81cf6
Compare
Build failed. ✔️ ansible-galaxy-importer SUCCESS in 3m 19s (non-voting) |
5c81cf6
to
b399656
Compare
Build failed. ❌ ansible-galaxy-importer FAILURE in 4m 02s (non-voting) |
return content | ||
raise TimeoutError(f"Unable to match expr '{expr}' from content") | ||
|
||
def flush_stderr(self) -> str: |
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.
Should we handle potential errors that may occur during the read process like OSError or TimeoutError?
|
||
|
||
class AnsibleAwsSSMSession: | ||
def __init__( |
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.
The constructor is quite long, and splitting it into smaller methods could improve readability. How do you feel about separating it into session management and socket handling?
def _socket_write(self, port: int, in_path: str) -> None: | ||
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as session: | ||
self._socket_connect(session=session, port=port) | ||
with open(in_path, "rb") as f: |
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.
Can we use a try-except block here?
Build failed. ✔️ ansible-galaxy-importer SUCCESS in 3m 25s (non-voting) |
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
SUMMARY
We are implementing the file transfer using a Port forwarding session to improve the velocity of the
aws_ssm
connection plugin.ISSUE TYPE
COMPONENT NAME
connection/aws_ssm