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.

Sign up for GitHub

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

Jump to bottom

Cleanup workflow templates; require 'with' key in module's .sync.yml #893

Merged
merged 3 commits into from
May 8, 2024
Merged

Cleanup workflow templates; require 'with' key in module's .sync.yml #893

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c5f3e04
Cleanup workflow templates; require 'with' key in module's .sync.yml
h-haaks May 4, 2024
0517b40
fix: only write hypens for string values
h-haaks May 5, 2024
71b96a7
fixup: even more cleanup
h-haaks May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions config_defaults.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
# yamllint disable rule:line-length
.github/workflows/ci.yml:
excludes: []
pidfile_workaround: false
additional_packages: ''
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Figured these to be unnecessary as it's the default value defined in the reusable workflows.
The next msync run will remove these inputs from ci.yml.

acceptance_tests: true
main_branches: ['main', 'master']
# PDK creates this
Expand Down
36 changes: 10 additions & 26 deletions moduleroot/.github/workflows/ci.yml.erb
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

I think it can be reduced even further. Sending a diff is tricky, so here's the code:

jobs:
  puppet:
    name: Puppet
<%- if @configs['acceptance_tests'] && Dir[File.join(@metadata[:workdir], 'spec', 'acceptance', '**', '*_spec.rb')].any? -%>
    uses: voxpupuli/gha-puppet/.github/workflows/beaker.yml@v2
<%- else -%>
    uses: voxpupuli/gha-puppet/.github/workflows/basic.yml@v2
<%- end -%>
<%- if @configs['with'] -%>
    with:
<%- @configs['with'].each do |k,v| -%>
<%- if v.is_a?(String) -%>
      <%= k %>: '<%= v %>'
<%- else -%>
      <%= k %>: <%= v %>
<%- end -%>
<%- end -%>
<%- end -%>

You can even use v.inspect to avoid the if/else:

<%- @configs['with'].each do |k,v| -%>
      <%= k %>: <%= v.inspect %>
<%- end -%>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

inspect returns a double quoted string. That will generate lots of diffs/changes in the next msync run i guess.

irb(main):001:0> t = 'test'
=> "test"
irb(main):002:0> t.inspect
=> "\"test\""

Copy link
Member

Choose a reason for hiding this comment

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

Oh yes, I may have confused it with Python where repr(my_string) is actually using single quotes.

Original file line number Diff line number Diff line change
Expand Up @@ -21,42 +21,26 @@ jobs:
name: Puppet
<%- if @configs['acceptance_tests'] && Dir[File.join(@metadata[:workdir], 'spec', 'acceptance', '**', '*_spec.rb')].any? -%>
uses: voxpupuli/gha-puppet/.github/workflows/beaker.yml@v2
with:
pidfile_workaround: '<%= @configs['pidfile_workaround'] %>'
<%- if @configs['with'] -%>
with:
<%- @configs['with'].each do |k,v| -%>
<%= k %>: <%= v %>
<%- end -%>
<%- end -%>
<%- if @configs['unit_runs_on'] -%>
unit_runs_on: '<%= @configs['unit_runs_on'] %>'
<%- end -%>
<%- if @configs['beaker_facter'] -%>
beaker_facter: '<%= @configs['beaker_facter'] %>'
<%- if v.is_a?(String) -%>
<%= k %>: '<%= v %>'
<%- else -%>
<%= k %>: <%= v %>
<%- end -%>
<%- if @configs['beaker_hypervisor'] -%>
beaker_hypervisor: '<%= @configs['beaker_hypervisor'] %>'
<%- end -%>
<%- if @configs['acceptance_runs_on'] -%>
acceptance_runs_on: '<%= @configs['acceptance_runs_on'] %>'
<%- end -%>
<%- else -%>
uses: voxpupuli/gha-puppet/.github/workflows/basic.yml@v2
<%- if @configs.key?('rubocop') || !@configs['additional_packages'].empty? || @configs.key?('unit_runs_on') || @configs.key?('with') -%>
with:
<%- end -%>
<%- if @configs['with'] -%>
with:
<%- @configs['with'].each do |k,v| -%>
<%= k %>: <%= v %>
<%- end -%>
<%- end -%>
<%- if v.is_a?(String) -%>
<%= k %>: '<%= v %>'
<%- else -%>
<%= k %>: <%= v %>
<%- end -%>
<%- unless @configs['additional_packages'].empty? -%>
additional_packages: '<%= @configs['additional_packages'] %>'
<%- end -%>
<%- if @configs.key?('rubocop') -%>
rubocop: <%= @configs['rubocop'] %>
<%- end -%>
<%- if Dir[File.join(@metadata[:workdir], 'spec', 'acceptance', '**', '*_spec.rb')].none? && @configs['unit_runs_on'] -%>
unit_runs_on: '<%= @configs['unit_runs_on'] %>'
<%- end -%>
8 changes: 7 additions & 1 deletion moduleroot/.github/workflows/release.yml.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,16 @@ jobs:
with:
<%- if @configs['with'] -%>
<%- @configs['with'].each do |k,v| -%>
<%= k %>: <%= v %>
<%- if v.is_a?(String) -%>
<%= k %>: '<%= v %>'
<%- else -%>
<%= k %>: <%= v %>
<%- end -%>
<%- end -%>
<%- end -%>
<%- if @configs['with'].nil? || !@configs['with'].has_key?('allowed_owner') -%>
Copy link
Member

Choose a reason for hiding this comment

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

This code is indeed intended to prevent running on forks. Isn't this equivalent?

Suggested change
<%- if @configs['with'].nil? || !@configs['with'].has_key?('allowed_owner') -%>
<%- unless @configs['with']&.has_key?('allowed_owner') -%>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, advanced ruby ...
I'll give it a try :)

allowed_owner: '<%= @configs[:namespace] %>'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Figure this is whats blocking forks from releasing.
Added a test to prevent adding it double if declared in a modules .sync.yml

<%- end -%>
secrets:
# Configure secrets here:
# https://docs.github.com/en/actions/security-guides/encrypted-secrets
Expand Down