Fixed deletion of invalid Access Log Policies

[xWink]

What type of PR is this?

bug

Which issue does this PR fix:
N/A

What does this PR do / Why do we need it:
Currently, when an Access Log Policy (ALP) has its spec changed such that it becomes invalid, it cannot be deleted because the controller validation short circuits to setting the ALP status to Invalid and doesn't remove finalizers nor delete the underlying Access Log Subscription (ALS).

This change removes the short circuit from the deletion logic so that ALPs with invalid specs and their underlying ALS can be deleted.

If an issue # is not available please add repro steps and logs from aws-gateway-controller showing the issue:

1. Create valid ALP
   • ALP status should be Accepted
2. Update ALP spec to have invalid Group (e.g. foo) or no destinationArn in targetRef
   • ALP status should be Invalid
3. Delete ALP
   • OLD: ALP deletion hangs because finalizers are not removed
   • NEW: ALP deletion succeeds and underlying ALS is deleted

Testing done on this change:

Reran all Access Log Policy e2e tests

Ran 13 of 51 Specs in 738.077 seconds
SUCCESS! -- 13 Passed | 0 Failed | 0 Pending | 38 Skipped
--- PASS: TestIntegration (739.34s)
PASS
ok      github.com/aws/aws-application-networking-k8s/test/suites/integration   740.098s

Automation added to e2e:

N/A, covered with unit tests

Will this PR introduce any new dependencies?:

No

Will this break upgrades or downgrades. Has updating a running cluster been tested?:

No

Does this PR introduce any user-facing change?:

No, this is part of the Access Log Policy feature release

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[coveralls]

Pull Request Test Coverage Report for Build 6737155513

• 9 of 24 (37.5%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.01%) to 44.179%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/gateway/model_build_access_log_subscription.go	9	11	81.82%
controllers/accesslogpolicy_controller.go	0	13	0.0%

Totals
Change from base Build 6722356691:	0.01%
Covered Lines:	3886
Relevant Lines:	8796

---

💛 - Coveralls

[mikhail-aws]

I dont have good suggestion yet, but it bothers me that we do conditional code branching on every step - reconciler, model, synthezier. Every time we do something that checks if it's delete/update/create, makes it harder to reason about how it actually works.

I'm thinking that create/update/delete branching should happen at Reconcile function. The rest of the code should not have these. It might create code duplicates, but those can be written into smaller functions. Right now I have to look at every stage and assemble little pieces of what is delete logic and build some mental map of it.

It's not a blocker, but I believe we come back to this code soon.

[xWink]

Agreed, I think this is going to be related to #439 and #463