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

Rsponholtz patch 1 #480

Closed
wants to merge 8 commits into from
Closed

Conversation

rsponholtz
Copy link
Member

Problem

When retrieving SSH keys from keyvault, the end of line character at the end was removed, resulting in an invalid key format. It appears that this is true for Ansible 2.8+. If earlier versions of ansible are still supported, this may cause an issue.

Solution

Ansible 2.8 adds the stdin_add_newline and strip_empty_ends to the ansible.builtin.command module: https://docs.ansible.com/ansible/devel/collections/ansible/builtin/command_module.html

please set stdin_add_newline to be true, since you can have newlines at the end of a private key, and set
strip_empty_ends to be false.

while stdin_add_newline should default to true, and strip_empty_ends also defaults to true according to the doc, the command will actially strip the newline from the end if neither of those parameters are set.

Tests

create a workspace dir, create the sap-parameters.yaml in that with the following content:

sap_fqdn: sap.contoso.net
kv_name: rheeee-9e91-2ea147f176e5
secret_prefix: RHE
orchestration_ansible_user: root
sap_sid: RHE

use the kv that you're actually using. log onto azure CLI with

az login

run the ansible using

ansible-playbook -e _workspace_directory=/root/ansible-test/workspace-dir ./pb_get-sshkey.yaml

Notes

since these options were added in ansible 2.8, if earlier versions are supported it may be nessesary to check the ansible version and use/not use the options

Kimmo Forss and others added 8 commits June 14, 2023 21:43
* Provide several fixes for Terraform code

* Fix internet check

* Fix owner of BOM directories

* Ignore PyCache directories

* Remove become from SDAF deployer check

* Bugfix dns_info_loadbalancers output variable for app_tier

* Add become to App installation block for Linux

* Create compact-sap-c++-10 symlink owner and group on the symlink, not the src file

The /opt/rh/SAP/lib64/compat-sap-c++-10.so src is provided by a Red Hat RPM. The owner / group should stay the same.

* sapmnt shouldn't be created on HANA systems

---------

Co-authored-by: Marges, RSY (Rick) <[email protected]>
Co-authored-by: Harm Jan Stam <[email protected]>
* Remove unsupported warn parameter from tasks

The warn parameter isn't supported anymore with newer Ansible versions.

* Bugfix grub config file content

The lookup module is executed on the controller and not on the remote host. Replace the lookup with a slurp.

* Bugfix BOM aggregate only find local bom folders

* Refactor curl command for Azure instance metadata to Ansible uri module

The Ansible uri module recognizes the json output returned by the azure metadata instance api. No converting to json is therefore needed anymore.

* Refactor anydb_node dns_info_vms output

* Remove how file
…#476)

* Fix DB2 for installation to lowercase db2sid folder

* Add input API variable for ABAP connect user
the previous code would strip the end-of-line character on the end of the private key from keyvault, resulting in the automation failing to ssh to the managed servers.
These options were added in ansible 2.8: https://docs.ansible.com/ansible/devel/collections/ansible/builtin/command_module.html
@rsponholtz rsponholtz requested a review from a team as a code owner August 15, 2023 15:48
@rsponholtz rsponholtz requested review from ajaygit158 and removed request for a team August 15, 2023 15:48
@hdamecharla
Copy link
Member

Hi @rsponholtz , thank you for the PR. Could you target the experimental branch for the PR please.

@hdamecharla
Copy link
Member

These changes should be included in SDAF - Release 3.9.0

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.

4 participants