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

Bridge and IGMP setting #106

Closed
durandguru opened this issue Feb 16, 2023 · 18 comments · Fixed by #125
Closed

Bridge and IGMP setting #106

durandguru opened this issue Feb 16, 2023 · 18 comments · Fixed by #125
Assignees
Labels
bug Something isn't working released

Comments

@durandguru
Copy link
Contributor

Describe the bug
I enable IGMP on the bridge. After apply I get an error about MikrotikResourceDataToTerraform

To Reproduce
resource "routeros_bridge" "bridge" {
name = "bridge"
ingress_filtering = true
protocol_mode = "rstp"
priority = "0x3000"
igmp_snooping = true
vlan_filtering = true
}

Expected behavior
A clear and concise description of what you expected to happen.

Additional context
Error
│ Warning: Field not found

│ with routeros_bridge.bridge,
│ on ports.tf line 1, in resource "routeros_bridge" "bridge":
│ 1: resource "routeros_bridge" "bridge" {

│ [MikrotikResourceDataToTerraform] The field was lost during the Schema development: igmp_version <<<= 2

@durandguru durandguru added the bug Something isn't working label Feb 16, 2023
@durandguru
Copy link
Contributor Author

Second error
│ Warning: Field not found

│ with routeros_bridge.bridge,
│ on ports.tf line 1, in resource "routeros_bridge" "bridge":
│ 1: resource "routeros_bridge" "bridge" {

│ [MikrotikResourceDataToTerraform] The field was lost during the Schema development: last_member_query_count <<<= 2

@durandguru
Copy link
Contributor Author

image
I Think al these are not in the bridge setting

@vaerh
Copy link
Collaborator

vaerh commented Feb 16, 2023

Wow, this menu appears after enabling IGMP, I did not see it when implementing a bridge interface and it will need to be added.

@gfenn-newbury
Copy link
Collaborator

Yep, here's the different between the API objects for two bridges - one has IGMP snooping enabled, the other doesn't:

[
  {
    ".id": "*2",
    "actual-mtu": "1500",
    "ageing-time": "5m",
    "arp": "enabled",
    "arp-timeout": "auto",
    "auto-mac": "true",
    "dhcp-snooping": "false",
    "disabled": "false",
    "fast-forward": "true",
    "forward-delay": "15s",
    "igmp-snooping": "false",
    "l2mtu": "65535",
    "mac-address": "FE:F3:52:67:CA:3A",
    "max-message-age": "20s",
    "mtu": "auto",
    "name": "bridge",
    "priority": "0x8000",
    "protocol-mode": "rstp",
    "running": "true",
    "transmit-hold-count": "6",
    "vlan-filtering": "false"
  },
  {
    ".id": "*4",
    "actual-mtu": "1500",
    "ageing-time": "5m",
    "arp": "enabled",
    "arp-timeout": "auto",
    "auto-mac": "true",
    "dhcp-snooping": "false",
    "disabled": "false",
    "fast-forward": "false",
    "forward-delay": "15s",
    "igmp-snooping": "true",
    "igmp-version": "2",
    "l2mtu": "65535",
    "last-member-interval": "1s",
    "last-member-query-count": "2",
    "mac-address": "7E:5F:A1:0E:25:56",
    "max-message-age": "20s",
    "membership-interval": "4m20s",
    "mld-version": "1",
    "mtu": "auto",
    "multicast-querier": "false",
    "multicast-router": "temporary-query",
    "name": "bridge-lan",
    "priority": "0x8000",
    "protocol-mode": "rstp",
    "querier-interval": "4m15s",
    "query-interval": "2m5s",
    "query-response-interval": "10s",
    "running": "true",
    "startup-query-count": "2",
    "startup-query-interval": "31s250ms",
    "transmit-hold-count": "6",
    "vlan-filtering": "false"
  }
]

I just tested setting one of the extra fields on a bridge that doesn't have IGMP snooping enabled, and it seems the API ignores it:

❯ curl -k -X PATCH -d '{"multicast-querier":"false"}' -H "content-type: application/json" https://localhost/rest/interface/bridge/\*2 -u admin
Enter host password for user 'admin':
{".id":"*2","actual-mtu":"1500","ageing-time":"5m","arp":"enabled","arp-timeout":"auto","auto-mac":"true","dhcp-snooping":"false","disabled":"false","fast-forward":"true","forward-delay":"15s","igmp-snooping":"false","l2mtu":"65535","mac-address":"FE:F3:52:67:CA:3A","max-message-age":"20s","mtu":"auto","name":"bridge","priority":"0x8000","protocol-mode":"rstp","running":"true","transmit-hold-count":"6","vlan-filtering":"false"

The only bad thing about this is if someone sets one of these igmp fields without actually enabling igmp snooping, Terraform will think it was successful and update the state. Additionally, I don't know of a way to enable/disable fields based on the value of another.

@vaerh
Copy link
Collaborator

vaerh commented Feb 17, 2023

Wait a bit, I'm preparing patches for this.

@vaerh
Copy link
Collaborator

vaerh commented Feb 17, 2023

The only bad thing about this is if someone sets one of these igmp fields without actually enabling igmp snooping, Terraform will think it was successful and update the state. Additionally, I don't know of a way to enable/disable fields based on the value of another.

Yes, this is predictable behavior, but I don't see a problem with it yet (storing dummy properties in the TF state file).
You can use predefined SDKv2 validators to control when creating a plan, but they do not allow you to control incorrect behavior if, for example, snooping is disabled and there are properties related to it.

igmp_snooping = false
igmp_version = 3

@vaerh vaerh closed this as completed in dd9baaa Feb 17, 2023
@vaerh vaerh reopened this Feb 17, 2023
@vaerh
Copy link
Collaborator

vaerh commented Feb 17, 2023

@durandguru Please check the fixes v1.0.7

@github-actions
Copy link

🎉 This issue has been resolved in version 1.0.7 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@terraform-routeros terraform-routeros deleted a comment from github-actions bot Feb 17, 2023
@durandguru
Copy link
Contributor Author

in 1.0.8

│ Warning: Field not found

│ with routeros_bridge.bridge,
│ on ports.tf line 1, in resource "routeros_bridge" "bridge":
│ 1: resource "routeros_bridge" "bridge" {

│ [MikrotikResourceDataToTerraform] The field was lost during the Schema development: multicast_router <<<=
│ temporary-query

@vaerh
Copy link
Collaborator

vaerh commented Feb 20, 2023

Hmm, I thought I found all the fields according to the documentation 😕. Please show your bridge section from the TF file.

vaerh added a commit that referenced this issue Feb 20, 2023
fix: #106 Added "multicast_router" field.
@durandguru
Copy link
Contributor Author

resource "routeros_bridge" "bridge" {
name = "bridge"
ingress_filtering = true
protocol_mode = "rstp"
priority = "0x1000"
igmp_snooping = true
vlan_filtering = true
}

I just use the default values from mikrotik for igmp.

@github-actions
Copy link

🎉 This issue has been resolved in version 1.0.9 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@durandguru
Copy link
Contributor Author

@vaerh tested 1.0.9 and got this error:

│ Error: InternalValidate

│ with provider["registry.terraform.io/gnewbury1/routeros"],
│ on provider.tf line 1, in provider "routeros":
│ 1: provider "routeros" {

│ Internal validation of the provider failed! This is always a bug
│ with the provider itself, and not a user issue. Please report
│ this bug:

│ 2 errors occurred:
│ * resource routeros_bridge: multicast_router: Default must be nil if computed
│ * resource routeros_interface_bridge: multicast_router: Default must be nil if computed

vaerh added a commit that referenced this issue Feb 20, 2023
vaerh added a commit that referenced this issue Feb 20, 2023
fix(#106): Fix internal validation (for release).
@github-actions
Copy link

🎉 This issue has been resolved in version 1.0.11 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@vaerh
Copy link
Collaborator

vaerh commented Feb 20, 2023

I'm sorry, I've corrected the error.

@durandguru
Copy link
Contributor Author

Almost all the errors are gone in 1.0.11.

Still got this one on routeros_bridge_port
Warning: Field not found

│ with routeros_bridge_port.ether1,
│ on ports.tf line 11, in resource "routeros_bridge_port" "ether1":
│ 11: resource "routeros_bridge_port" "ether1" {

│ [MikrotikResourceDataToTerraform] The field was lost during the Schema development: root_path_cost <<<= 10

vaerh added a commit that referenced this issue Feb 22, 2023
This is one of the 8 fields for monitoring the current state of the bridge.
@vaerh vaerh mentioned this issue Feb 22, 2023
vaerh added a commit that referenced this issue Feb 22, 2023
This is one of the 8 fields for monitoring the current state of the bridge.
@github-actions
Copy link

🎉 This issue has been resolved in version 1.0.12 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@vaerh vaerh reopened this Feb 22, 2023
@durandguru
Copy link
Contributor Author

These messages are gone ;-)

@vaerh vaerh closed this as completed Feb 22, 2023
gfenn-newbury pushed a commit that referenced this issue Feb 23, 2023
This is one of the 8 fields for monitoring the current state of the bridge.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment