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

change firewall recent.time to string as per VyOS 1.4 #313

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

change firewall recent.time to string as per VyOS 1.4 #313

wants to merge 8 commits into from

Conversation

athiedeilluminate
Copy link

SUMMARY

Possibly Fixes #312

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

Changing the type of a single variable.

@softwarefactory-project-zuul
Copy link

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/324c69c406b34ec48de086ee116af6ce

✔️ ansible-galaxy-importer SUCCESS in 3m 22s
✔️ build-ansible-collection SUCCESS in 8m 26s
❌ ansible-test-network-integration-vyos-paramiko-python39-devel NODE_FAILURE Node request 200-0005873161 failed in 0s
❌ ansible-test-network-integration-vyos-paramiko-python39-stable214 NODE_FAILURE Node request 200-0005873162 failed in 0s
❌ ansible-test-network-integration-vyos-paramiko-python39-stable213 NODE_FAILURE Node request 200-0005873163 failed in 0s
❌ ansible-test-network-integration-vyos-paramiko-python39-stable212 NODE_FAILURE Node request 200-0005873164 failed in 0s
❌ ansible-test-network-integration-vyos-paramiko-python36-stable29 NODE_FAILURE Node request 200-0005873165 failed in 0s (non-voting)
❌ ansible-test-network-integration-vyos-libssh-python39-devel NODE_FAILURE Node request 200-0005873166 failed in 0s
❌ ansible-test-network-integration-vyos-libssh-python39-stable214 NODE_FAILURE Node request 200-0005873167 failed in 0s
❌ ansible-test-network-integration-vyos-libssh-python39-stable213 NODE_FAILURE Node request 200-0005873168 failed in 0s
❌ ansible-test-network-integration-vyos-libssh-python39-stable212 NODE_FAILURE Node request 200-0005873169 failed in 0s
❌ ansible-test-network-integration-vyos-libssh-python36-stable29 NODE_FAILURE Node request 200-0005873170 failed in 0s
❌ ansible-tox-linters FAILURE in 10m 23s

@sever-sever
Copy link
Member

It's a moot point if you integrate it for the 1.4 rolling release; it will be broken for the stable 1.3 release, which still uses int

vyos@r1# set firewall name EXTERNAL-LOCAL rule 30 recent time 
Possible completions:
   <0-4294967295>
                Source addresses seen in the last N seconds

Are there any options to use some args for modules like version 1.3 or 1.4, any other ideas?

@athiedeilluminate
Copy link
Author

I just started using this today, so I'm not very familiar with the codebase, but I can look into that. It would be nice to have the option. Or it could just be a freetext field?

Qalthos
Qalthos suggested changes Apr 12, 2023
View reviewed changes
Copy link
Contributor

@Qalthos Qalthos left a comment

Choose a reason for hiding this comment

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

We technically only support vyos 1.1.x at the moment, but getting on to something newer is something that I have been wanting for a while, so if this works well enough to support newer versions in a transitional way, I'm all for it.

Can you please add a changelog fragment to your PR? Thanks!

plugins/module_utils/network/vyos/argspec/firewall_rules/firewall_rules.py Outdated Show resolved Hide resolved
plugins/modules/vyos_firewall_rules.py Outdated Show resolved Hide resolved
@athiedeilluminate
Copy link
Author

I've added a changelog fragment and removed the "choices" and left it as a string since I think that will help it remain backwards-compatible.

@athiedeilluminate athiedeilluminate changed the title change firewall recent.count to string as per VyOS 1.4 change firewall recent.time to string as per VyOS 1.4 Apr 12, 2023
@sever-sever
Copy link
Member

We technically only support vyos 1.1.x at the moment, but getting on to something newer is something that I have been wanting for a while, so if this works well enough to support newer versions in a transitional way, I'm all for it.

The thing is 1.1.x EOL
1.2 is legacy LTS (could be downloaded here https://vyos.net/get/#1.2.9-S1)
1.3 LTS
1.4 Rolling/developing branch (https://vyos.net/get/nightly-builds/)

I guess it is better to open a new topic and discuss there capability and other options :)

@softwarefactory-project-zuul
Copy link

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/0c54ba4f6d9340a696c3088f65e940f4

⚠️ ansible-galaxy-importer SKIPPED Skipped due to failed job build-ansible-collection
❌ build-ansible-collection POST_FAILURE in 8m 13s
⚠️ ansible-test-network-integration-vyos-paramiko-python39-devel SKIPPED Skipped due to failed job build-ansible-collection
⚠️ ansible-test-network-integration-vyos-paramiko-python39-stable214 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ ansible-test-network-integration-vyos-paramiko-python39-stable213 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ ansible-test-network-integration-vyos-paramiko-python39-stable212 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ ansible-test-network-integration-vyos-paramiko-python36-stable29 SKIPPED Skipped due to failed job build-ansible-collection (non-voting)
⚠️ ansible-test-network-integration-vyos-libssh-python39-devel SKIPPED Skipped due to failed job build-ansible-collection
⚠️ ansible-test-network-integration-vyos-libssh-python39-stable214 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ ansible-test-network-integration-vyos-libssh-python39-stable213 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ ansible-test-network-integration-vyos-libssh-python39-stable212 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ ansible-test-network-integration-vyos-libssh-python36-stable29 SKIPPED Skipped due to failed job build-ansible-collection
✔️ ansible-tox-linters SUCCESS in 10m 37s

@dmbaturin dmbaturin requested a review from c-po August 12, 2024 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Qalthos Qalthos Qalthos requested changes

@c-po c-po Awaiting requested review from c-po

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

VyOS 1.4 changed firewall rule 'recent time' to second/minute/hour from decimal (n seconds)
3 participants
@athiedeilluminate @sever-sever @Qalthos