Add support for custom egress rules

[CecileRobertMichon]

What type of PR is this?

What this PR does / why we need it:

• Renames ingressRule to securityRules (note there was a missing s in the field name which represented a list of ingress rules) to be more consistent with Azure naming, and to include rules in both directions
• Adds a direction field to SecurityRule which can be either Inbound or Outbound. Defaults to Inbound to preserve existing behavior.

Feature request came out of this slack thread

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes partly #618

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

• squashed commits
• includes documentation
• adds unit tests

Release note:

Rename ingressRule to securityRules
Add optional direction field to securityRules, defaults to "Inbound"

[cpanato]

/lgtm

[whites11]

For what it's worth LGTM

[CecileRobertMichon]

/hold for squash

[shysank]

Isn't this logic redundant since inbound rules will get auto converted (now that you've set direction as well in line 263)? IIUC, I think we can simplify this to just handle outbound rules:

if restoredSecurityRule.Direction == infrav1alpha4.SecurityRuleDirectionOutbound {
  restoredOutboundRules = append(restoredOutboundRules, restoredSecurityRule)
}

wdyt?

[CecileRobertMichon]

You are right! I had actually thought of that earlier and thought it would break the Fuzzy test because the directions set in the fuzzer are random strings. But, what I ended up doing is:

if restoredSecurityRule.Direction != infrav1alpha4.SecurityRuleDirectionInbound {
  restoredOutboundRules = append(restoredOutboundRules, restoredSecurityRule)
}

Which makes the Fuzzer happy, and, IMO, is more logically correct because the condition for "dropping" a rule in v1alpha4 --> v1alpha3 conversion is "if direction != Inbound" (we only include rules where direction == Ibound). So it makes sense to restore all rules that aren't inbound since they would all have been dropped theoretically speaking. And in the unlikely event that we need to add a new direction in v1alpha4 (for example, "both") we don't need to change the existing conversion.

[shysank]

+1 checking for != inbound instead of ==outbound, more future proof!

[k8s-ci-robot]

@CecileRobertMichon: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-cluster-api-provider-azure-apidiff	715d5fd	link	/test pull-cluster-api-provider-azure-apidiff

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[CecileRobertMichon]

/hold cancel

[shysank]

/lgtm

[CecileRobertMichon]

/assign @devigned

[devigned]

/lgtm
/appove

Looks great! Always love seeing docs updates with a PR 😎.

[CecileRobertMichon]

/approve

:D

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [CecileRobertMichon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment