-
Notifications
You must be signed in to change notification settings - Fork 1
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
Implement FWaaS for ASR1k #106
Conversation
25f23fb
to
1ce67c7
Compare
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.
First high-level review pass from my side. Overall I like it!
asr1k_neutron_l3/plugins/l3/service_plugins/l3_extension_adapter.py
Outdated
Show resolved
Hide resolved
asr1k_neutron_l3/plugins/l3/service_plugins/l3_extension_adapter.py
Outdated
Show resolved
Hide resolved
asr1k_neutron_l3/neutron/services/service_drivers/asr1k/fwaas_driver.py
Outdated
Show resolved
Hide resolved
2fc57a0
to
88a8339
Compare
88a8339
to
9f31e74
Compare
7a9ad12
to
13c2fa9
Compare
asr1k_neutron_l3/neutron/services/service_drivers/asr1k/fwaas_driver.py
Outdated
Show resolved
Hide resolved
73db3e7
to
c088626
Compare
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.
I like it, cool implementation. Looking forward of having this in prod.
asr1k_neutron_l3/plugins/l3/service_plugins/l3_extension_adapter.py
Outdated
Show resolved
Hide resolved
asr1k_neutron_l3/plugins/l3/service_plugins/l3_extension_adapter.py
Outdated
Show resolved
Hide resolved
If specified as a service_provider for FIREWALL_V2 the ASR1KFWaaSDriver will receive all pre/postcommit hooks necessary to implement the OpenStack FWaasV2 service.
fwg policies are shared objects, they can be used by multiple routers. We map them 1:1 to ACL objects on IOS XE and thus need a method to keep them updated. We could do that by calling a router sync on any single router that has a reference to that policy or we just call it on an arbitrary router on every agent. This is what we implement in this commit.
In our FWaaS implementation we will most likely bind policies on external interfaces. In order to tell the customer the external interface we expand the API to convey this information.
In order for the ZBF feature to replicate the state table, interfaces need to be assigned to a redundandancy group and receive a Redundant Interface Identifier (RII) in order to find the same interface on the remote side. In case we find an external policy on the `Router`, we mark all interfaces on the router's interface list with `has_stateful_firewall`. If that is the case, we instantiate the yang model with RII, redundancy group membership and in case of the interface being the gateway interface with an zone membership. As this marking is done post `Interface` instantiation, we make the `_rest_definition` attribute a dynamic property, so later changes in the `Interface` object are represented in its `_rest_definition`.
c088626
to
8c4d1a8
Compare
As we cannot support hardware stateful firewalling, we mimic stateful behaviour by inserting a set of rules of the beginning of each ACL that help us letting through packets that are usually an answer to a sent out packet.
7a6528a
to
44fdc9f
Compare
All netconf_yang classes inherit their `is_orphan` from BulkOperations. Each of these subclasses need different lists to determine if the classes object is orphan or not. Let's simplify adding arguments to this method by using keyword arguments. All subclasses shall send unnecessary arguments to `*args, **kwargs` while only `BulkOperations.is_orphan` should know about the whole argument list.
Zones and ZonePairs are created per VRF that has an external policy attached. We can remove them straigh away once the external policy was removed on an VRF/external router interface.
FWaaS rules are mapped via policies and firewall groups to ports. Yet when we need to troubelshoot, we must compare it to a virtual router's configuration, so we'd need easy access to how the rules would look from a router interface perspective. This commit adds an API endpoint that delivers that view, removing some cluttering attributes from the result.
I noticed that the first iterations of all loopingcalls would always fail. Turns out, `_run_yang_cmd` checks the `alive` condition of the context, which is not set until `device_check_loop` has run the first time. To make matters worse, `device_check_loop` is also the last loopingcall to be triggered, so it's certain that the first iteration off all previous calls will fail. I moved the loopingcall to be the first one triggered and also halt until we see the agents marked alive. I believe that makes sense as no yang action can be run before this happens either way. As I don't want to inhibit agent startup alltogether, I still back out after `nc_timeout`.
44fdc9f
to
75252d7
Compare
Firewall policies can be bound to multiple ports. They are realized by ACLs, a ClassMap and a ServicePolicy (if the policy is bound to an external interface). That means, just because a router or a port is removed from a policy the corresponding objects may still be used from other routers. Hence it is not removed immediatly but by a cleanup loop, that checks if anybody on an agent is using that policy and then it will be removed.
75252d7
to
9980f5f
Compare
This prevents cyclic imports as the prefix is needed in the orphan check, but also needed when creating the object through one of the higher level l3 classes.
To remove clutter from the AccessList class we move all the code that generates the attributes for the corresponding yang class into `Rule`.
9980f5f
to
39fc6fb
Compare
In the previous implementation `firewall.AccessList` would inherit from `l3.AccessList` which inherited from `base.Base`. `firewall.AccessList` also inherited from `firewall.FirewallPolicyObject`. That lead to a Diamond Shaped inheritance. The fact that `FirewallPolicyObject` and `l3.AccessList` had constructors with different arguments made it difficult to let `super()` call the constructors with different arguments. To resolve that, the diamond Shaped inheritance was resolved making it clearer which `__init__`'s `super()` calls which superclasses constructor. The different signatures of the constructor still pose a problem, hence we send all supplemental arguments of a constructor to `**kwargs`.
39fc6fb
to
07dc92f
Compare
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.
Nice work.
is_orphan
should_be_none
all the way through