Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Added option to not removing $encrypted$ from filetree_create exported credentials #791

Merged
merged 4 commits into from
Mar 21, 2024

Conversation

ecchong
Copy link
Contributor

@ecchong ecchong commented Mar 20, 2024

What does this PR do?

Added option to not removing $encrypted$ from filetree_create exported credentials. It keeps the $encrypted$ text, instead of `` empty string, in the credential files. It prevents the existing credential value from being overwritten with empty string if the configure files are being used as-is to import back into AAP.

A condition is added to roles/filetree_create/templates/current_credentials.j2 to control the output.

How should this be tested?

Include show_encrypted: true in task invoking filetree_create

- name: Export AAP credentials for organization {{ org_name }}
  ansible.builtin.import_role:
    name: infra.controller_configuration.filetree_create
  vars:
    organization_filter: "{{ org_name }}"
    output_path: "{{ _objects_output_dir }}"
    flatten_output: false
    input_tag:
      - credentials
    show_encrypted: true

Observe password fileds in output files

---
controller_credentials:
  - name: "SSH root"
    description: "Test"
    credential_type: "Machine"
    organization: "Default"
    inputs:
      become_method: ''
      become_username: ''
      ssh_key_data: $encrypted$
      ssh_key_unlock: $encrypted$
      username: root

Is there a relevant Issue open for this?

resolves #788

Other Relevant info, PRs, etc

N/A

@adonisgarciac
Copy link
Contributor

Hi, I'd prefer keeping $encrypted$ text as default in order to prevent remove things unintentional. @ivarmu what do you think?

@ivarmu
Copy link
Contributor

ivarmu commented Mar 21, 2024

My opinion is that, if having the $encrypted$ as the sensitive data isn't causing any modifications to the controller objects when applied, it could give a false sensation of having the configuration completely ok, when it's not true at all... in case of a migration or complete disaster, the configuration as code wouldn't be as complete as expected... and the solution should be to populate the sensible data, as with the current approach.

That said... I'd prefer giving the option to leave the $encrypted$ if anyone is interested in it, but no for the default value.

Does it makes sense?

Copy link
Contributor

@ivarmu ivarmu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ivarmu ivarmu enabled auto-merge (squash) March 21, 2024 14:31
@ivarmu ivarmu merged commit 35e9f78 into redhat-cop:devel Mar 21, 2024
13 checks passed
przemkalit pushed a commit to przemkalit/aap_configuration that referenced this pull request Nov 22, 2024
…d credentials (redhat-cop#791)

* Added option to not removing $encrypted$ from filetree_create exported credentials

* Added option to not removing $encrypted$ from filetree_create exported credentials

* Added option to not removing $encrypted$ from filetree_create exported credentials

---------

Co-authored-by: Ivan Aragonés Muniesa <26822043+ivarmu@users.noreply.github.com>
przemkalit pushed a commit to przemkalit/aap_configuration that referenced this pull request Nov 22, 2024
…d credentials (redhat-cop#791)

* Added option to not removing $encrypted$ from filetree_create exported credentials

* Added option to not removing $encrypted$ from filetree_create exported credentials

* Added option to not removing $encrypted$ from filetree_create exported credentials

---------

Co-authored-by: Ivan Aragonés Muniesa <26822043+ivarmu@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should filetree_create export the credential with '$encrypted$' instead of ''?
3 participants