-
Notifications
You must be signed in to change notification settings - Fork 127
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
feat(lb): add support for frontend ACL #382
Conversation
Hi @alekc Thank you very much for your contribution 👍 . In the instance product we do something like this:
For ACL this would translate to
This notation as multiple advantages:
Do you want to work on this on this PR or do you prefer we merge it as is. Knowing that we will implement the above solution before the next release? |
Ok, I will amend the current pr to align it to security groups. |
First draft for realignment. I need to write a better testing, but otherwise it's almost there I'd say (still need to build the provider and try to run some create/update/destroy by hand to). This is a full example resource scaleway_lb_frontend_beta frt01 {
lb_id = scaleway_lb_beta.lb01.id
backend_id = scaleway_lb_backend_beta.bkd01.id
name = "tf-test"
inbound_port = 80
timeout_client = "30s"
acl {
name = "test-acl"
action {
type = "allow"
}
match {
ip_subnet = ["192.168.0.1", "192.168.0.2", "192.168.10.0/24"]
http_filter = "acl_http_filter_none"
http_filter_value = ["criteria1","criteria2"]
invert = "true"
}
}
acl {
action {
type = "allow"
}
match {
http_filter = "path_begin"
http_filter_value = ["criteria1","criteria2"]
invert = "true"
}
}
acl {
action {
type = "allow"
}
match {
http_filter_value = ["criteria1","criteria2"]
}
}
acl {
action {
type = "allow"
}
match {
http_filter = "acl_http_filter_none"
http_filter_value = ["criteria1","criteria2"]
}
}
acl {
match {
ip_subnet = ["192.168.0.1", "192.168.0.2", "192.168.10.0/24"]
http_filter = "acl_http_filter_none"
http_filter_value = ["criteria1","criteria2"]
invert = "true"
}
action {
type = "deny"
}
}
} Some considerations:
acl {
action_type = "allow/deny"
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, great work 👏 . I added a few remarks.
@kindermoumoute can you please also have a look?
Thanks @jerome-quere , I will amend pull request. Meanwhile I am having some issues with update tests because I think there is a bug with scaleway api
Notice the and from api documentation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@kindermoumoute or @jerome-quere can you verify if there is indeed a bug with the scaleway api as described in https://github.com/terraform-providers/terraform-provider-scaleway/pull/382#issuecomment-575088706? I don't think there is a public place for those kinds of reports. |
The issue has been escalated to the load-balancer team, I'll let you know as soon as we have more information. |
@alekc @jerome-quere Fixed ! Sorry about that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
👏 |
This commit adds support for scaleway lb acl (https://developers.scaleway.com/en/products/lb/api/#get-ff2ff4)
I followed scaleway official api, so tf structure looks like this
potentially we can transform action.type in action_type, but it's probably best to keep it aligned to official apis.
Tests results