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 system hooks management #380

Merged
merged 11 commits into from
May 8, 2021
Merged

Conversation

cduelo
Copy link
Contributor

@cduelo cduelo commented May 7, 2021

A file hook will run on each event so it's up to you to filter events
or projects within a file hook code. You can have as many file hooks
as you want. Each file hook will be triggered by GitLab asynchronously
in case of an event. For a list of events see the system hooks
documentation.
Reference: http://10.191.3.4/help/administration/file_hooks

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

cduelo added 2 commits May 7, 2021 11:35
A file hook will run on each event so it's up to you to filter events
or projects within a file hook code. You can have as many file hooks
as you want. Each file hook will be triggered by GitLab asynchronously
in case of an event. For a list of events see the system hooks
documentation.
Reference: http://10.191.3.4/help/administration/file_hooks
manifests/init.pp Outdated Show resolved Hide resolved
manifests/init.pp Outdated Show resolved Hide resolved
@bastelfreak bastelfreak self-assigned this May 7, 2021
@bastelfreak bastelfreak added the enhancement New feature or request label May 7, 2021
@bastelfreak bastelfreak removed their assignment May 7, 2021
@bastelfreak
Copy link
Member

thanks for the PR! I made some tiny inline comments, can you please take a look?

cduelo and others added 6 commits May 7, 2021 15:02
@cduelo
Copy link
Contributor Author

cduelo commented May 7, 2021

@bastelfreak for your help! I think I resolved all the your comments

Boolean $config_manage = true,
Stdlib::Absolutepath $config_file = '/etc/gitlab/gitlab.rb',
Optional[Hash] $consul = undef,
Optional[String] $custom_hooks_dir = '/opt/gitlab/embedded/service/gitlab-shell/hooks',
Copy link
Member

Choose a reason for hiding this comment

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

since those two variables now have a default value, they cannot be Optional (because you cannot pass undef to it).
when when we already update it we can update the datatype as well

Suggested change
Optional[String] $custom_hooks_dir = '/opt/gitlab/embedded/service/gitlab-shell/hooks',
Stdlib::Absolutepath $custom_hooks_dir = '/opt/gitlab/embedded/service/gitlab-shell/hooks',

Stdlib::Absolutepath $config_file = '/etc/gitlab/gitlab.rb',
Optional[Hash] $consul = undef,
Optional[String] $custom_hooks_dir = '/opt/gitlab/embedded/service/gitlab-shell/hooks',
Optional[Stdlib::Absolutepath] $system_hooks_dir = '/opt/gitlab/embedded/service/gitlab-rails/file_hooks',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Optional[Stdlib::Absolutepath] $system_hooks_dir = '/opt/gitlab/embedded/service/gitlab-rails/file_hooks',
Stdlib::Absolutepath $system_hooks_dir = '/opt/gitlab/embedded/service/gitlab-rails/file_hooks',

aligning the = is quite impossible in this code suggestion :D

# @param content Specify the system hook contents either as a string or using the template function. If this paramter is specified source parameter must not be present.
# @param source Specify a file source path to populate the system hook contents. If this paramter is specified content parameter must not be present.
define gitlab::system_hook (
Stdlib::Absolutepath $system_hooks_dir,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Stdlib::Absolutepath $system_hooks_dir,
Stdlib::Absolutepath $system_hooks_dir = $gitlab::system_hooks_dir,

if we do this we don't need the if/else logic in line 21-25

@bastelfreak
Copy link
Member

thanks for all the work!

@bastelfreak bastelfreak merged commit 48cc366 into voxpupuli:master May 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants