Preserve external controller state during reconciliation

[ciarams87]

Proposed changes

Problem: NGINX Gateway Fabric's provisioner removes all annotations during service reconciliation that aren't explicitly managed by NGF. This causes conflicts with external controllers that need to add operational annotations.

Solution: Implemented a preservation pattern for both service annotations by merging Service annotations instead of overwriting

Testing: Unit testing, manual testing in progress

Please focus on (optional): If you any specific areas where you would like reviewers to focus their attention or provide
specific feedback, add them here.

Closes #4012
Closes #4175

Checklist

Before creating a PR, run through this checklist and mark each as complete.

• I have read the CONTRIBUTING doc
• I have added tests that prove my fix is effective or that my feature works
• I have checked that all unit tests pass after adding my changes
• I have updated necessary documentation
• I have rebased my branch onto main
• I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.

Preserve external controller annotations 

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.00%. Comparing base (dc0007d) to head (1e19f96).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4182      +/-   ##
==========================================
- Coverage   86.00%   86.00%   -0.01%     
==========================================
  Files         131      131              
  Lines       14111    14115       +4     
  Branches       35       35              
==========================================
+ Hits        12136    12139       +3     
- Misses       1772     1773       +1     
  Partials      203      203              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sjberman]

@ciarams87 Do we really need a new API field for preserving annotations? Going off of this comment, could we essentially ignore or automatically preserve any existing fields set in the metadata.annotations/labels? And only add what we need, instead of overwriting?

Maybe this requires a CreateOrPatch instead of CreateOrUpdate that we currently use?

[ciarams87]

@ciarams87 Do we really need a new API field for preserving annotations? Going off of this comment, could we essentially ignore or automatically preserve any existing fields set in the metadata.annotations/labels? And only add what we need, instead of overwriting?

Maybe this requires a CreateOrPatch instead of CreateOrUpdate that we currently use?

@sjberman You're right, I was over complicating it.

I'll update the implementation

[bjee19]

Does there still exist a way to remove annotations if we start with all existing annotations then overwrite with NGF-managed annotations?

[ciarams87]

Does there still exist a way to remove annotations if we start with all existing annotations then overwrite with NGF-managed annotations?

@bjee19 Great question - you're right - there's a limitation: if a user sets an annotation via Gateway.Spec.Infrastructure.Annotations or NginxProxy patches and later removes it from the Gateway spec, it won't be automatically removed from the Service. It can still be removed by manually deleting it, but it won't be cleaned up automatically.

The proper fix is to migrate to Server-Side Apply, which tracks field ownership automatically. I think migrating to SSA is too large a change for a patch release since it affects how we reconcile all resources, so I think we should go with the approach and accept the limitation for now, but I'm going to create a follow up ticket to move to SSA instead. What do you think?

[sjberman]

@ciarams87 How large of a change do you think it really is? We'd technically be introducing a new bug, where a user removes some configuration, but then we don't actually delete it.