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

Various improvements required to connect to a managed remote host #65

Merged
merged 26 commits into from Aug 18, 2022

Conversation

badnetmask
Copy link
Contributor

@badnetmask badnetmask commented Aug 10, 2022

Use case

Ansible host is a regular application server running RHEL 8, and needs to establish an IPSec connection to a NetApp storage. The storage is managed by various other means (either Ansible or other proprietary tools), but requires very specific parameters in order to establish a connection.

This pull request implements the necessary methods to pass all the required parameters to the tunnel configuration.

Features that have been added

  • When establishing a connection between the current running host on a playbook, and only one remote host, you don't need to specify the host in the connection variable.
  • The flag no_log has been added to the beginning of the opportunistic configuration to prevent leak of the PSK.
  • A few tweaks on the README file.

Options that have been added

  • shared_key_content -- the PSK in plain text (added a note to README about secrecy of the value).
  • leftid and rightid -- the IPSec ID of each host (when different from the FQDN).
  • ike_enc_alg and esp_enc_alg -- Allow detailed specification of the encryption algorithms.
  • type -- Tunnel or transport.
  • retransmit_timeout -- IKE packet re-transmit timeout.

@richm
Copy link
Contributor

richm commented Aug 11, 2022

[citest commit:7966f6e9f5925d73bcf634fa4e478698865a321b]

@richm
Copy link
Contributor

richm commented Aug 15, 2022

ok - #66 was merged - please rebase

@richm
Copy link
Contributor

richm commented Aug 15, 2022

Can you do a rebase without a merge commit?

@richm
Copy link
Contributor

richm commented Aug 16, 2022

I created another PR

  • removes the merge commit
  • fixes an error with if tunnel.hosts[host2]['shared_key_content']
  • adds a test for shared_key_content

#68

@badnetmask
Copy link
Contributor Author

Sorry, when I saw your rebase note I had already merged. What should I do now?

@richm
Copy link
Contributor

richm commented Aug 16, 2022

Sorry, when I saw your rebase note I had already merged. What should I do now?

one of these

@richm
Copy link
Contributor

richm commented Aug 16, 2022

[citest]

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@richm
Copy link
Contributor

richm commented Aug 16, 2022

If we accept this PR, we're going to need a follow up PR to add testing for the new parameters, otherwise, QE is going to be very unhappy, and/or we won't be able to put this functionality into the downstream product. @ueno can you help write some tests for leftid/rightid, ike_enc_alg, esp_enc_alg, and type?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
use ascii double quotes
tasks/main.yml Show resolved Hide resolved
richm and others added 3 commits August 17, 2022 16:08
Add the test tests_host_to_host_psk_custom.yml to test for the
optional parameters.

Update the documentation for the new parameters.
Fix typo.
add test for new parameters; update docs
@richm
Copy link
Contributor

richm commented Aug 18, 2022

[citest]
At least tests_host_to_host_psk_custom.yml should pass

@richm
Copy link
Contributor

richm commented Aug 18, 2022

[citest]

@richm richm merged commit c80969d into linux-system-roles:master Aug 18, 2022
@badnetmask badnetmask deleted the various_improvements branch August 23, 2022 11:31
richm added a commit to richm/linux-system-roles-vpn that referenced this pull request Sep 19, 2022
[1.3.6] - 2022-09-19
--------------------

### New Features

- Various improvements required to connect to a managed remote host (linux-system-roles#65)

Add support for the parameters shared_key_content, leftid, rightid, ike, esp, type, ikelifetime, salifetime, retransmit_timeout, dpddelay, dpdtimeout, dpdaction, leftupdown

### Bug Fixes

- Check for /usr/bin/openssl on controller - do not use package_facts (linux-system-roles#66)

Check for existence of openssl without using sudo on the controller
Basically, any task, even package_facts:, will use sudo if using
become=true - so just use "exists" test to check for /usr/bin/openssl

### Other Changes

- Fix a bash bug in changelog_to_tag.yml, which unexpectedly expanded "*" (linux-system-roles#62)

- changelog_to_tag action - support other than "master" for the main branch name, as well (linux-system-roles#63)

- Use GITHUB_REF_NAME as name of push branch; fix error in branch detection [citest skip] (linux-system-roles#64)

We need to get the name of the branch to which CHANGELOG.md was pushed.
For now, it looks as though `GITHUB_REF_NAME` is that name.  But don't
trust it - first, check that it is `main` or `master`.  If not, then use
a couple of other methods to determine what is the push branch.

Signed-off-by: Rich Megginson <rmeggins@redhat.com>
richm added a commit to richm/linux-system-roles-vpn that referenced this pull request Sep 19, 2022
[1.4.0] - 2022-09-19
--------------------

- Various improvements required to connect to a managed remote host (linux-system-roles#65)

Add support for the parameters shared_key_content, leftid, rightid, ike, esp, type, ikelifetime, salifetime, retransmit_timeout, dpddelay, dpdtimeout, dpdaction, leftupdown

- Check for /usr/bin/openssl on controller - do not use package_facts (linux-system-roles#66)

Check for existence of openssl without using sudo on the controller
Basically, any task, even package_facts:, will use sudo if using
become=true - so just use "exists" test to check for /usr/bin/openssl

- Fix a bash bug in changelog_to_tag.yml, which unexpectedly expanded "*" (linux-system-roles#62)

- changelog_to_tag action - support other than "master" for the main branch name, as well (linux-system-roles#63)

- Use GITHUB_REF_NAME as name of push branch; fix error in branch detection [citest skip] (linux-system-roles#64)

We need to get the name of the branch to which CHANGELOG.md was pushed.
For now, it looks as though `GITHUB_REF_NAME` is that name.  But don't
trust it - first, check that it is `main` or `master`.  If not, then use
a couple of other methods to determine what is the push branch.

Signed-off-by: Rich Megginson <rmeggins@redhat.com>
richm added a commit that referenced this pull request Sep 19, 2022
[1.4.0] - 2022-09-19
--------------------

- Various improvements required to connect to a managed remote host (#65)

Add support for the parameters shared_key_content, leftid, rightid, ike, esp, type, ikelifetime, salifetime, retransmit_timeout, dpddelay, dpdtimeout, dpdaction, leftupdown

- Check for /usr/bin/openssl on controller - do not use package_facts (#66)

Check for existence of openssl without using sudo on the controller
Basically, any task, even package_facts:, will use sudo if using
become=true - so just use "exists" test to check for /usr/bin/openssl

- Fix a bash bug in changelog_to_tag.yml, which unexpectedly expanded "*" (#62)

- changelog_to_tag action - support other than "master" for the main branch name, as well (#63)

- Use GITHUB_REF_NAME as name of push branch; fix error in branch detection [citest skip] (#64)

We need to get the name of the branch to which CHANGELOG.md was pushed.
For now, it looks as though `GITHUB_REF_NAME` is that name.  But don't
trust it - first, check that it is `main` or `master`.  If not, then use
a couple of other methods to determine what is the push branch.

Signed-off-by: Rich Megginson <rmeggins@redhat.com>

Signed-off-by: Rich Megginson <rmeggins@redhat.com>
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.

None yet

2 participants