search

puppetlabs / puppetlabs-peadm

A Puppet module defining Bolt plans used to automate Puppet Enterprise deployments
Apache License 2.0
29 stars 53 forks source link

Make $pe_installer_source more flexible #465

Open bastelfreak opened 1 month ago

bastelfreak commented 1 month ago

This makes the peadm::install plan more flexible. Previously the variable $pe_installer_source could point to an absolute URL directly to the desired .tar.gz. Now it can also point to a web directory. Then peadm will calculate the .tar.gz name and fetch it from the web directory.

A simple test for the peadm::upgrade plan can be done with the following parameters:

{
  "primary_host": "$fqdn",
  "version": "2023.7.0",
  "pe_installer_source": "https://s3.amazonaws.com/pe-builds/released/2023.7.0/"
}

Summary

Provide a detailed description of all the changes present in this pull request.

Additional Context

Add any additional context about the problem here.

Related Issues (if any)

Mention any related issues or pull requests.

Checklist

Changes include test coverage?

Have you updated the documentation?

bastelfreak commented 1 month ago

I haven't tested this yet, but a PE user asked me to implement it.

bastelfreak commented 1 month ago

I did some tests for upgrade and install and this works fine for me. @timidri is there a chance you could merge and release this soonish?

timidri commented 1 month ago

I did some tests for upgrade and install and this works fine for me. @timidri is there a chance you could merge and release this soonish?

Hi @bastelfreak, I am not the code owner, so the team does the ultimate merging and releasing.

Question: what problem is solved by this change? /cc @GSPatton

bastelfreak commented 1 month ago

The problem is that Puppet doesn't deliver a public mirror for PE releases. See: https://github.com/puppetlabs/puppet-enterprise_issues/issues/46

Because my customers cannot mirror it in an automated way, they download the installers and put them all in one directory that's served via HTTPS. The different PE instances (around 3000) don't have access to the internet, only to internal mirrors. That's where PEADM will fetch it from. With this patch I can just provide PEADM a URL to the directory, it will calculate the archive name and fetch it from there.

I implemented this, instead of providing an absolute url to the archive, because the archives contain the OS major version (like el-9) and my customers run different operating systems. Figuring out the correct name before calling PEADM is a bit annoying.

timidri commented 1 month ago

@bastelfreak Thank you for the clarifications. This feature seems like a useful addition for a quite specific use case. @GSPatton - what do you think?

From the arch/tech perspective, I would recommend to:

bastelfreak commented 1 month ago

This should be good to go now.

bastelfreak commented 1 month ago

@CoMfUcIoS can you please take another look again? CI is fine, and further debugging in #469 showed that this PR isn't related to it.

bastelfreak commented 2 weeks ago

Hi people, any chance this can be merged soonish? Is there anything missing?

bastelfreak commented 1 week ago

@CoMfUcIoS can you please take another look?

bastelfreak commented 4 days ago

@GSPatton do you have an update for me?

GSPatton commented 3 days ago

Hi @bastelfreak. Thank you for your contribution and your patience. We're currently working to get time with our internal security team to evaluate any potential security risks associated with this. As you can imagine, security is a top priority for us and we want to ensure a thorough review before proceeding.

I'll keep you updated on our progress and let you know as soon as we have more information.

Thanks again for your understanding!

Gavin