Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@gemmahou
gemmahou committed Oct 4, 2024
1 parent 9f74d67 commit 275eca1
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 169 deletions.
266 changes: 98 additions & 168 deletions mockgcp/mockcompute/firewallpoliciesv1.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,76 +82,7 @@ func (s *FirewallPoliciesV1) Insert(ctx context.Context, req *pb.InsertFirewallP

// Use default rules
if obj.Rules == nil {
obj.Rules = []*pb.FirewallPolicyRule{
{
Action: PtrTo("goto_next"),
Description: PtrTo("default egress rule ipv6"),
Direction: PtrTo("EGRESS"),
EnableLogging: PtrTo(false),
Kind: PtrTo("compute#firewallPolicyRule"),
Match: &pb.FirewallPolicyRuleMatcher{
DestIpRanges: []string{"::/0"},
Layer4Configs: []*pb.FirewallPolicyRuleMatcherLayer4Config{
{
IpProtocol: PtrTo("all"),
},
},
},
Priority: PtrTo(int32(2147483644)),
RuleTupleCount: PtrTo(int32(2)),
},
{
Action: PtrTo("goto_next"),
Description: PtrTo("default ingress rule ipv6"),
Direction: PtrTo("INGRESS"),
EnableLogging: PtrTo(false),
Kind: PtrTo("compute#firewallPolicyRule"),
Match: &pb.FirewallPolicyRuleMatcher{
SrcIpRanges: []string{"::/0"},
Layer4Configs: []*pb.FirewallPolicyRuleMatcherLayer4Config{
{
IpProtocol: PtrTo("all"),
},
},
},
Priority: PtrTo(int32(2147483645)),
RuleTupleCount: PtrTo(int32(2)),
},
{
Action: PtrTo("goto_next"),
Description: PtrTo("default egress rule"),
Direction: PtrTo("EGRESS"),
EnableLogging: PtrTo(false),
Kind: PtrTo("compute#firewallPolicyRule"),
Match: &pb.FirewallPolicyRuleMatcher{
DestIpRanges: []string{"0.0.0.0/0"},
Layer4Configs: []*pb.FirewallPolicyRuleMatcherLayer4Config{
{
IpProtocol: PtrTo("all"),
},
},
},
Priority: PtrTo(int32(2147483646)),
RuleTupleCount: PtrTo(int32(2)),
},
{
Action: PtrTo("goto_next"),
Description: PtrTo("default ingress rule"),
Direction: PtrTo("INGRESS"),
EnableLogging: PtrTo(false),
Kind: PtrTo("compute#firewallPolicyRule"),
Match: &pb.FirewallPolicyRuleMatcher{
SrcIpRanges: []string{"0.0.0.0/0"},
Layer4Configs: []*pb.FirewallPolicyRuleMatcherLayer4Config{
{
IpProtocol: PtrTo("all"),
},
},
},
Priority: PtrTo(int32(2147483647)),
RuleTupleCount: PtrTo(int32(2)),
},
}
populateDefaultRules(obj)
}

if err := s.storage.Create(ctx, fqn, obj); err != nil {
Expand Down Expand Up @@ -197,7 +128,7 @@ func (s *FirewallPoliciesV1) Patch(ctx context.Context, req *pb.PatchFirewallPol
// patch operation finished super fast
Progress: PtrTo(int32(100)),
Status: PtrTo(pb.Operation_DONE),
EndTime: PtrTo("2024-04-01T12:34:56.123456Z"),
EndTime: PtrTo(s.nowString()),
}
return s.startGlobalOrganizationLRO(ctx, op, func() (proto.Message, error) {
return obj, nil
Expand Down Expand Up @@ -242,81 +173,13 @@ func (s *FirewallPoliciesV1) GetRule(ctx context.Context, req *pb.GetRuleFirewal
return nil, err
}

// When the target policy has no rules, i.e. all the custom rules are deleted,
// we update the policy to add default rules to it.
if obj.Rules == nil {
// add default rule
obj.Rules = []*pb.FirewallPolicyRule{
{
Action: PtrTo("goto_next"),
Description: PtrTo("default egress rule ipv6"),
Direction: PtrTo("EGRESS"),
EnableLogging: PtrTo(false),
Kind: PtrTo("compute#firewallPolicyRule"),
Match: &pb.FirewallPolicyRuleMatcher{
DestIpRanges: []string{"::/0"},
Layer4Configs: []*pb.FirewallPolicyRuleMatcherLayer4Config{
{
IpProtocol: PtrTo("all"),
},
},
},
Priority: PtrTo(int32(2147483644)),
RuleTupleCount: PtrTo(int32(2)),
},
{
Action: PtrTo("goto_next"),
Description: PtrTo("default ingress rule ipv6"),
Direction: PtrTo("INGRESS"),
EnableLogging: PtrTo(false),
Kind: PtrTo("compute#firewallPolicyRule"),
Match: &pb.FirewallPolicyRuleMatcher{
SrcIpRanges: []string{"::/0"},
Layer4Configs: []*pb.FirewallPolicyRuleMatcherLayer4Config{
{
IpProtocol: PtrTo("all"),
},
},
},
Priority: PtrTo(int32(2147483645)),
RuleTupleCount: PtrTo(int32(2)),
},
{
Action: PtrTo("goto_next"),
Description: PtrTo("default egress rule"),
Direction: PtrTo("EGRESS"),
EnableLogging: PtrTo(false),
Kind: PtrTo("compute#firewallPolicyRule"),
Match: &pb.FirewallPolicyRuleMatcher{
DestIpRanges: []string{"0.0.0.0/0"},
Layer4Configs: []*pb.FirewallPolicyRuleMatcherLayer4Config{
{
IpProtocol: PtrTo("all"),
},
},
},
Priority: PtrTo(int32(2147483646)),
RuleTupleCount: PtrTo(int32(2)),
},
{
Action: PtrTo("goto_next"),
Description: PtrTo("default ingress rule"),
Direction: PtrTo("INGRESS"),
EnableLogging: PtrTo(false),
Kind: PtrTo("compute#firewallPolicyRule"),
Match: &pb.FirewallPolicyRuleMatcher{
SrcIpRanges: []string{"0.0.0.0/0"},
Layer4Configs: []*pb.FirewallPolicyRuleMatcherLayer4Config{
{
IpProtocol: PtrTo("all"),
},
},
},
Priority: PtrTo(int32(2147483647)),
RuleTupleCount: PtrTo(int32(2)),
},
}
if err := s.storage.Update(ctx, fqn, obj); err != nil {
return nil, err
}
populateDefaultRules(obj)
}
if err := s.storage.Update(ctx, fqn, obj); err != nil {
return nil, err
}

var rule *pb.FirewallPolicyRule
Expand Down Expand Up @@ -349,17 +212,7 @@ func (s *FirewallPoliciesV1) AddRule(ctx context.Context, req *pb.AddRuleFirewal
}

r := req.GetFirewallPolicyRuleResource()
// RuleTupleCount is output only, calculation of the complexity of a single firewall policy rule.
// Manually set different ruleTupleCount to match the realGCP log
if r.TargetResources != nil {
r.RuleTupleCount = PtrTo(int32(4))
} else {
r.RuleTupleCount = PtrTo(int32(2))
}
r.Kind = PtrTo("compute#firewallPolicyRule")
if r.Description == nil {
r.Description = PtrTo("")
}
mockFieldValuesForRule(r)

obj.Rules = []*pb.FirewallPolicyRule{r}
if err := s.storage.Update(ctx, fqn, obj); err != nil {
Expand Down Expand Up @@ -397,17 +250,7 @@ func (s *FirewallPoliciesV1) PatchRule(ctx context.Context, req *pb.PatchRuleFir
// update the rule
r := req.GetFirewallPolicyRuleResource()
r.Priority = PtrTo(*rule.Priority)
// RuleTupleCount is output only, calculation of the complexity of a single firewall policy rule.
// Manually set different ruleTupleCount to match the realGCP log
if r.TargetResources != nil {
r.RuleTupleCount = PtrTo(int32(4))
} else {
r.RuleTupleCount = PtrTo(int32(2))
}
r.Kind = PtrTo("compute#firewallPolicyRule")
if r.Description == nil {
r.Description = PtrTo("")
}
mockFieldValuesForRule(r)
rules = append(rules, r)
} else {
rules = append(rules, rule)
Expand Down Expand Up @@ -482,7 +325,7 @@ func (n *firewallPolicyName) String() string {
func (s *MockService) parseFirewallPolicyName(name string) (*firewallPolicyName, error) {
tokens := strings.Split(name, "/")

if len(tokens) == 4 && tokens[2] == "firewallPolicies" {
if len(tokens) == 4 && tokens[0] == "locations" && tokens[1] == "global" && tokens[2] == "firewallPolicies" {
name := &firewallPolicyName{
Name: tokens[3],
}
Expand All @@ -491,3 +334,90 @@ func (s *MockService) parseFirewallPolicyName(name string) (*firewallPolicyName,
return nil, status.Errorf(codes.InvalidArgument, "name %q is not valid", name)
}
}

func populateDefaultRules(obj *pb.FirewallPolicy) {
obj.Rules = []*pb.FirewallPolicyRule{
{
Action: PtrTo("goto_next"),
Description: PtrTo("default egress rule ipv6"),
Direction: PtrTo("EGRESS"),
EnableLogging: PtrTo(false),
Kind: PtrTo("compute#firewallPolicyRule"),
Match: &pb.FirewallPolicyRuleMatcher{
DestIpRanges: []string{"::/0"},
Layer4Configs: []*pb.FirewallPolicyRuleMatcherLayer4Config{
{
IpProtocol: PtrTo("all"),
},
},
},
Priority: PtrTo(int32(2147483644)),
RuleTupleCount: PtrTo(int32(2)),
},
{
Action: PtrTo("goto_next"),
Description: PtrTo("default ingress rule ipv6"),
Direction: PtrTo("INGRESS"),
EnableLogging: PtrTo(false),
Kind: PtrTo("compute#firewallPolicyRule"),
Match: &pb.FirewallPolicyRuleMatcher{
SrcIpRanges: []string{"::/0"},
Layer4Configs: []*pb.FirewallPolicyRuleMatcherLayer4Config{
{
IpProtocol: PtrTo("all"),
},
},
},
Priority: PtrTo(int32(2147483645)),
RuleTupleCount: PtrTo(int32(2)),
},
{
Action: PtrTo("goto_next"),
Description: PtrTo("default egress rule"),
Direction: PtrTo("EGRESS"),
EnableLogging: PtrTo(false),
Kind: PtrTo("compute#firewallPolicyRule"),
Match: &pb.FirewallPolicyRuleMatcher{
DestIpRanges: []string{"0.0.0.0/0"},
Layer4Configs: []*pb.FirewallPolicyRuleMatcherLayer4Config{
{
IpProtocol: PtrTo("all"),
},
},
},
Priority: PtrTo(int32(2147483646)),
RuleTupleCount: PtrTo(int32(2)),
},
{
Action: PtrTo("goto_next"),
Description: PtrTo("default ingress rule"),
Direction: PtrTo("INGRESS"),
EnableLogging: PtrTo(false),
Kind: PtrTo("compute#firewallPolicyRule"),
Match: &pb.FirewallPolicyRuleMatcher{
SrcIpRanges: []string{"0.0.0.0/0"},
Layer4Configs: []*pb.FirewallPolicyRuleMatcherLayer4Config{
{
IpProtocol: PtrTo("all"),
},
},
},
Priority: PtrTo(int32(2147483647)),
RuleTupleCount: PtrTo(int32(2)),
},
}
}

func mockFieldValuesForRule(r *pb.FirewallPolicyRule) {
// RuleTupleCount is output only, calculation of the complexity of a single firewall policy rule.
// Manually set different ruleTupleCount to match the realGCP log
if r.TargetResources != nil {
r.RuleTupleCount = PtrTo(int32(4))
} else {
r.RuleTupleCount = PtrTo(int32(2))
}
r.Kind = PtrTo("compute#firewallPolicyRule")
if r.Description == nil {
r.Description = PtrTo("")
}
}
1 change: 0 additions & 1 deletion tests/e2e/unified_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,6 @@ func runScenario(ctx context.Context, t *testing.T, testPause bool, fixture reso
// Matches the mock ip address of Compute forwarding rule
addReplacement("IPAddress", "8.8.8.8")
addReplacement("pscConnectionId", "111111111111")
addReplacement("pscConnectionId", "111111111111")

// Extract resource targetID numbers from compute operations
for _, event := range events {
Expand Down

0 comments on commit 275eca1

Please sign in to comment.