Skip to content
This repository has been archived by the owner on Nov 24, 2021. It is now read-only.

[WIP] Refs #25377 - Refactor certs tar check into a generic absolute paths check #717

Closed
wants to merge 2 commits into from

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Nov 26, 2018

This hook allows converting relative paths into absolute ones. The file presence is done in the actual puppet module and thus redundant.

Depends on theforeman/puppet-certs#230

This hook allows converting relative paths into absolute ones. The file
presence is done in the actual puppet module and thus redundant.
@theforeman-bot

This comment has been minimized.

@theforeman-bot
Copy link

There were the following issues with the commit message:

  • 1f3078f must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

@ekohl ekohl changed the title Refs #25377 - Refactor certs tar check into a generic absolute paths check [WIP] Refs #25377 - Refactor certs tar check into a generic absolute paths check Nov 26, 2018
@ekohl
Copy link
Member Author

ekohl commented Nov 26, 2018

Added a commit that I have not tested but might solve it in a much cleaner way.

Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

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

@ekohl tested and the post action didnt remove the value in the answer file :(

[root@dhcp182-46 ~]# yum install -y foreman-proxy-content
[root@dhcp182-46 katello-installer-base]# wget https://patch-diff.githubusercontent.com/raw/Katello/katello-installer/pull/717.patch
[root@dhcp182-46 katello-installer-base]# patch -p1 < 717.patch 
patching file hooks/pre_validations/11-absolute_paths.rb
patching file hooks/pre_validations/12-check_capsule_tar.rb
patching file hooks/post/20-clean-answers.rb

[root@dhcp182-46 ~]# ll /root

-rw-r--r--. 1 root root 64568 Nov 26 14:06 certs.tar

[root@dhcp182-46 ~]# foreman-installer --scenario foreman-proxy-content\
>                     --foreman-proxy-content-parent-fqdn           "foreman.toledo.satellite.lab.eng.rdu2.redhat.com"\
>                     --foreman-proxy-register-in-foreman           "true"\
>                     --foreman-proxy-foreman-base-url              "https://foreman.toledo.satellite.lab.eng.rdu2.redhat.com"\
>                     --foreman-proxy-trusted-hosts                 "foreman.toledo.satellite.lab.eng.rdu2.redhat.com"\
>                     --foreman-proxy-trusted-hosts                 "dhcp182-46.gsslab.rdu2.redhat.com"\
>                     --foreman-proxy-oauth-consumer-key            "i3qm5fS66zHWSZgEJM9wFUYAYCXBnCS8"\
>                     --foreman-proxy-oauth-consumer-secret         "e3xNjAQDUFvw4YdidQPopsVVKF7MW6RR"\
>                     --foreman-proxy-content-certs-tar             "/root/certs.tar"\
>                     --puppet-server-foreman-url                   "https://foreman.toledo.satellite.lab.eng.rdu2.redhat.com"
Resetting puppet server version param...
Installing             Done                                               [100%] [....................................................................................................................................................................]
  Success!
  * Foreman Proxy is running at https://dhcp182-46.gsslab.rdu2.redhat.com:9090
  The full log is at /var/log/foreman-installer/foreman-proxy-content.log

[root@dhcp182-46 ~]# cat /etc/foreman-installer/scenarios.d/foreman-proxy-content-answers.yaml | grep certs_tar
  certs_tar: /root/certs.tar

I had issues using the mod method you are using too when working on my pr, and had to switch to pulling the param like f-p-c.certs_tar.value Is that something that only works with migrations?

@ekohl
Copy link
Member Author

ekohl commented Nov 27, 2018

@chris1984 I've pushed an untested updated version that uses the pre_values hook instead. I've also taken the Puppet values that should never be stored and made them part of the hook. I'm going to test it later, but feel free to verify it as well.

Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

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

Still seeing the certs tar answer being stored :(
After fixing the unset.value the installer moved past the error around the nil.

 Success!
  * Foreman Proxy is running at https://dhcp182-46.gsslab.rdu2.redhat.com:9090
  The full log is at /var/log/foreman-installer/foreman-proxy-content.log
yum install -y -q rh-mongodb34-syspaths finished successfully!

[root@dhcp182-46 ~]# cat /etc/foreman-installer/scenarios.d/foreman-proxy-content-answers.yaml | grep certs_tar
  certs_tar: /root/certs.tar


if puppetserver_metrics_param = param('puppet', 'server_puppetserver_metrics')
puppetserver_metrics_param.unset_value
end
Copy link
Member

Choose a reason for hiding this comment

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

These changes seem unrelated.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can refactor it in a separate commit: one that first introduced the always-clear hook and the next to apply it to the tar check. However, first I want to get it to work at all.

@@ -0,0 +1,11 @@
# These answers should never be stored
Copy link
Member

Choose a reason for hiding this comment

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

The idea is that these are just always going to be erased from the answers file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sort of. We can't prevent them from being written to the answers file and probably that's a good thing for debugging purposes so the idea is to unset the answers after loading the answers file. However, the current version doesn't work.

@ekohl ekohl changed the title [WIP] Refs #25377 - Refactor certs tar check into a generic absolute paths check Refs #25377 - Refactor certs tar check into a generic absolute paths check Dec 2, 2018
@ekohl ekohl changed the title Refs #25377 - Refactor certs tar check into a generic absolute paths check [WIP] Refs #25377 - Refactor certs tar check into a generic absolute paths check Dec 2, 2018
@ekohl
Copy link
Member Author

ekohl commented Dec 14, 2018

This repository has been deprecated and merged into https://github.com/theforeman/foreman-installer (#731). If this is still relevant, please resubmit the PR there.

@ekohl ekohl closed this Dec 14, 2018
@ekohl ekohl deleted the absolute-paths branch December 14, 2018 20:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants