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

Native firewalld custom service #277

Merged

Conversation

trevor-vaughan
Copy link
Collaborator

  • Create firewalld_custom_service type and provider
  • Add spec tests
  • Update Beaker tests
  • Mark firewalld::custom_service for deprecation
  • Resolve dependency loops

Fixes #275

@trevor-vaughan trevor-vaughan force-pushed the native_firewalld_custom_service branch from b2a7363 to 5d7924e Compare March 30, 2020 20:10
@trevor-vaughan
Copy link
Collaborator Author

Coverage went down even though it has more tests than it's ever had before. Not correcting at this time.

@alexjfisher
Copy link
Member

Coverage went down even though it has more tests than it's ever had before. Not correcting at this time.

That's not why the tests are failing though.

@trevor-vaughan
Copy link
Collaborator Author

Oh, I see. Got buried in the garbage.

@trevor-vaughan
Copy link
Collaborator Author

@alexjfisher OK, one more try :-|

@alexjfisher
Copy link
Member

I’d like to try this out one more time before merging. Probably tomorrow.

@alexjfisher
Copy link
Member

I've confirmed this is a breaking change for me. The following two resources worked before, but don't any more.

firewalld::custom_service { 'foreman-smart-proxy':
    filename    => 'foreman-smart-proxy',
    short       => 'Foreman Smart Proxy',
    description => 'Smart Proxy is a free open source project that provides restful API to subsystems such as DNS, DHCP, etc, for higher level orchestration tools such as Foreman.',
    port        => [
      {
        'port'     => '8443',
        'protocol' => 'tcp',
      },
    ],
  }
firewalld_service { 'Allow Foreman Smart Proxy from public zone':
    ensure  => 'present',
    service => 'foreman-smart-proxy',
    zone    => 'public',
}
Error: Execution of '/bin/firewall-cmd --permanent --zone public --add-service foreman-smart-proxy' returned 101: 
Error: /Stage[main]/Main/Firewalld_service[Allow Foreman Smart Proxy from public zone]/ensure: change from 'absent' to 'present' failed: Execution of '/bin/firewall-cmd --permanent --zone public --add-service foreman-smart-proxy' returned 101: 
Notice: /Stage[main]/Firewalld::Reload/Exec[firewalld::reload]: Dependency Firewalld_service[Allow Foreman Smart Proxy from public zone] has failures: true
Warning: /Stage[main]/Firewalld::Reload/Exec[firewalld::reload]: Skipping because of failed dependencies
Warning: /Stage[main]/Firewalld::Reload::Complete/Exec[firewalld::complete-reload]: Skipping because of failed dependencies
Notice: /Stage[main]/Main/Firewalld::Custom_service[foreman-smart-proxy]/Firewalld_custom_service[Foreman Smart Proxy]/ensure: created

@trevor-vaughan
Copy link
Collaborator Author

@alexjfisher Can you check this again when you have time please?

@trevor-vaughan trevor-vaughan added the bug Something isn't working label Apr 7, 2020
@trevor-vaughan
Copy link
Collaborator Author

@alexjfisher Did a full refactor and beefed up the acceptance tests significantly to cover (what I hope) is all of the use cases.

README.md Show resolved Hide resolved
REFERENCE.md Outdated Show resolved Hide resolved
Copy link
Member

@alexjfisher alexjfisher left a comment

Choose a reason for hiding this comment

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

LTGM. I can't break anything anymore. This should be squashed down to 1 (or just a few commits) before merging please.

* Create firewalld_custom_service type and provider
* Add spec tests
* Update Beaker tests
* Mark firewalld::custom_service for deprecation
* Resolve dependency loops

Fixes voxpupuli#275
@trevor-vaughan trevor-vaughan force-pushed the native_firewalld_custom_service branch from c8b5e67 to a1993ba Compare April 16, 2020 19:26
@alexjfisher alexjfisher merged commit 839f60e into voxpupuli:master Apr 16, 2020
@alexjfisher alexjfisher added enhancement New feature or request and removed bug Something isn't working labels Apr 16, 2020
@alexjfisher alexjfisher added this to the v4.3.0 milestone Apr 25, 2020
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.

The firewalld module has loop issues when chaining dependent class resources
3 participants