search

np-guard / vpc-network-config-analyzer

A tool for analyzing the configured network connectivity of VPCs as specified by various VPC resources
Apache License 2.0
7 stars 0 forks source link

Consider having one unified interface for the filters (SG/NACL) and other connection rules resources (transit gateway) #504

Closed ShiriMoran closed 1 month ago

ShiriMoran commented 4 months ago

Consider re-organizing functionality related merely to explian tgw, and merging similar structs for rules/prefixes indexes The function StringPrefixDetails is more explainability-specific... the relevant part from TGW to return is the prefilx filter object and its indexes. maybe should consider to separate those functionalities (also for filter traffic resources), consider a separate issue.. (also the prefix filter index is some other "rule" similar to rule index already captured for nacl/sg rules here... ) maybe the abstract type RulesInFilter should be modified to capture also rules for tgw objects?

_Originally posted by @adisos in https://github.com/np-guard/vpc-network-config-analyzer/pull/492#discussion_r1555352574_

ShiriMoran commented 4 months ago

(also the prefix filter index is some other "rule" similar to rule index already captured for nacl/sg rules here... ) maybe the abstract type RulesInFilter should be modified to capture also rules for tgw objects?

The tgw object equiv of RulesInFilter is not stored at the explainability struct. Only the relevant tgw router is stored at the struct; the deduction of the prefix filter here is more straightforward - no multiple tables and layers as in the filter case.

Having said that, it might make sense to expand existing structs to also include the prefix filter details. The issue with this solution is that at the moment the transit gateway is a RoutingResource while the SG and NACL are FilterTrafficResource. We should consider having a single interface for all resources having rules that effects the connection - one that will capture the above three resources. This can be done either by introducing a new interface or by having transit gateway implement FilterTrafficResource. Are there any other resources that might be relevant? This requires some discussion. For now I'm removing this issue from V0.4. Lets discuss @adisos @zivnevo

ShiriMoran commented 4 months ago

The function StringPrefixDetails is more explainability-specific... the relevant part from TGW to return is the prefilx filter object and its indexes. maybe should consider to separate those functionalities (also for filter traffic resources), consider a separate issue..

  1. Its not only the indexes, its also the lt gt properties and the action; it is internal to the transit gateway and I'm not sure we want to expose these specific details outside of the IBMVPC package
  2. In the filters case we use an internal (to SG/NACL) functionality to print - specifically, getSGRule and getNACLRule. This functionality is not only for explainability and we can use it, of course, only from within the internal SG and NACL functionality.
ShiriMoran commented 4 months ago

after discussing with @zivnevo and @adisos it was agreed to add an interface for filters that will be implemented by NACLLayer, SecurityGroupLayer and TransitGateway; functionality used by Explainability should be moved to this interface. Once this Interface is in place, it should make sense to add crossVPC rulesInLayers to rulesConnection

ShiriMoran commented 3 months ago

solved partly by #574 We will revisit once there are more rule based resources, as agreed with @adisos For now lowering priority and removing from V0.5

ShiriMoran commented 1 month ago

Consider doing it after https://github.com/np-guard/vpc-network-config-analyzer/pull/675 is merge, by defining an interface of []RuleOfFilter (define type) that will implement functionality to replace StringDetailsOfRules(...) and ListFilterWithAction(...)