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

Add an optional key_source parameter to set up the package repository. #35

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gitbensons
Copy link

Our internal hosts have no access to the internet, hence no option to download GPG-KEY-elasticsearch. This tiny patch adds the option to specify the download location for the GPG file.

This is needed for hosts without access to the internet.
@cla-checker-service
Copy link

❌ Author of the following commits did not sign a Contributor Agreement:
6aa14c5

Please, read and sign the above mentioned agreement if you want to contribute to this project

@bahner
Copy link

bahner commented Nov 19, 2021

Hepp! @gitbensons - do you have anything against signing the agreement? Otherwise I will steal your commit and repeat your work. This is important for me too.

@gitbensons
Copy link
Author

@bahner you are free to steal the commit 🙂

@smortex
Copy link
Member

smortex commented Nov 26, 2021

The agreement is not required anymore: the module is now managed by voxpupuli. The transition is in progress, once #39 is merged, the CI should stop checking this 😉

@smortex
Copy link
Member

smortex commented Nov 26, 2021

Can you please rebase your changes on top of the main branch?

From your working directory:

git fetch origin       # Download the latest code we have here
git rebase origin/master # Move your commits on top of the main branch
git push -f            # Send the changes (-f is required because we re-wrote history)

@gitbensons
Copy link
Author

@smortex I synced my repo with master and rebased my changes. Not sure if this actually did what you expected. Directly after pushing the changes, Github went down, so I take this as a sign :)

Copy link
Member

@smortex smortex left a comment

Choose a reason for hiding this comment

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

New CI proves to be useful \o/

Q: if your nodes cannot download the PGP key, how do they manage to fetch the actual packages? Are these changes really enough?

@@ -18,6 +18,7 @@
String $proxy = 'absent',
Integer $version = 7,
Optional[String] $base_repo_url = undef,
Optional[String] $key_source = 'https://artifacts.elastic.co/GPG-KEY-elasticsearch'
Copy link
Member

Choose a reason for hiding this comment

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

CI failure can be fixed by removing the Optional. While here, maybe we can use a stricter type than String, e.g. String[1] (non-empty string) or Stdlib::HTTPUrl (which would not allow puppet:///modules/profile/esGPG-KEY-elasticsearch/ so maybe not so good?). Also, missing coma at end of line 😉

Suggested change
Optional[String] $key_source = 'https://artifacts.elastic.co/GPG-KEY-elasticsearch'
Stdlib::HTTPUrl $key_source = 'https://artifacts.elastic.co/GPG-KEY-elasticsearch',

@bahner
Copy link

bahner commented Dec 23, 2021 via email

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.

3 participants