[v2.5] Re-add support for getambassador.io/v1

[LukeShu]

Description

There are 3 reasons why this PR is worth-while (ordered most-compelling first):

1. To resolve K8s control plane logs conversion webhook errors #4240; we'll discuss that below.
2. To port-over the daisy-chain conversion logic from Edge Stack, which will be valuable when we move past apiVersion v3alpha. I discuss this in more length at https://github.com/emissary-ingress/emissary/wiki/design-doc:-Emissary's-CRD-conversion-logic
3. To remove friction for users migrating from Emissary 1.x.

Why this fixes #4240

Background: Users coming from Emissary 1.x likely have resources stored as getambassador.io/v1. That is, the CRDs' status.storedVersions will include v1. In order to be able to even apply the newer crds.yaml, it must include all of the old status.storedVersions. So, the 2.1-2.4 crds.yaml includes an "unserved" v1 that is unusable by users, but satisfies Kubernetes' requirement that all versions in status.storedVersions be mentioned in the definition. (Thanks to @kflynn for reminding me of this background--I'd totally forgotten it.)

Related: https://www.notion.so/datawire/smooth-upgrades-how-v1-is-handled-aae77e4dcb554a6bafcdfc071875b48f

However, it turns out that even though it's unserved, Kube wants to ask apiext about it--even if it's a fresh 2.x/3.x install. That's what #4240. We try to could come up with ways to get Kube to stop asking about v1. But it's fairly easy to just actually add support for v1 so that we don't error when Kube asks about it; plus this has the 2 other benefits that it reduces friction for users and that we'll benefit from the machinery in the future, so even if it were harder than the alternative, it would likely still be worth it.

Related Issues

• K8s control plane logs conversion webhook errors #4240

Testing

I've verified that I don't see the no kind "XXX" is registered for version "getambassador.io/v1" errors from docker logs when running in k3d that @kflynn said he sees. But he should verify as well that this resolves things for his set up.

Checklist

• I made sure to update CHANGELOG.md.

  Remember, the CHANGELOG needs to mention:

  • Any new features
  • Any changes to our included version of Envoy
  • Any non-backward-compatible changes
  • Any deprecations

• This is unlikely to impact how Ambassador performs at scale. - I'd be shocked if any of the places I touch are in the hot-path, but there probably are more allocations now, so IDK

  Remember, things that might have an impact at scale include:

  • Any significant changes in memory use that might require adjusting the memory limits
  • Any significant changes in CPU use that might require adjusting the CPU limits
  • Anything that might change how many replicas users should use
  • Changes that impact data-plane latency/scalability

• My change is adequately tested. - pending feedback from @kflynn

  Remember when considering testing:

  • Your change needs to be specifically covered by tests.
    • Tests need to cover all the states where your change is relevant: for example, if you add a behavior that can be enabled or disabled, you'll need tests that cover the enabled case and tests that cover the disabled case. It's not sufficient just to test with the behavior enabled.
  • You also need to make sure that the entire area being changed has adequate test coverage.
    • If existing tests don't actually cover the entire area being changed, add tests.
    • This applies even for aspects of the area that you're not changing – check the test coverage, and improve it if needed!
  • We should lean on the bulk of code being covered by unit tests, but...
  • ... an end-to-end test should cover the integration points

• I updated DEVELOPING.md with any any special dev tricks I had to use to work on this code efficiently. - no new tricks

[LanceEa]

Thanks for tracking this down and addressing it so quickly. I will give it a deeper review on Monday, but at first glance it looks good. I left a few questions.

I think it would be good to test out @kflynn reproducer to verify it isn't occurring still and we will want to make sure this gets pulled into Edge Stack right away verify it doesn't have any issues (I don't think there will be)

[LanceEa]

Overall looks good, a couple of comments and questions.

1. @kflynn - it would be good to have you verify it
2. Do we need to pull this into Edge Stack and verify all is good before merging? My initial reaction is no because these are isolated to the apiext so we shouldn't have any issues.

[kflynn]

Looks good from here, and I can verify that it shuts up the logs for me, too. I think that @LanceEa is correct about the mostly-static scaffold file, but I think that's a good topic for further discussion and a future PR, rather than a necessary thing right now.

Anecdotally, I saw timeouts from apiext when testing this, which worries me a bit. Time to look into that, maybe.

[LukeShu]

2 changes:

• I've added a commit moving the AWK scripts to separate files.
• I've merged [v2.5] Bump the Python version #4683 to fix the build

[LukeShu]

Rebased on to #4685 to resolve linter issues

[LanceEa]

lgtm...Looks like this is landing in 2.5 so we will want to get it landed in master too.

[LukeShu]

Fixed the changelog:

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 884539fa1..302918068 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -82,8 +82,8 @@ it will be removed; but as it won't be user-visible this isn't considered a brea
 
 ## RELEASE NOTES
 
-## [2.5.0] November 03, 2022
-[2.5.0]: https://github.com/emissary-ingress/emissary/compare/v2.4.0...v2.5.0
+## [2.5.1] TBD
+[2.5.1]: https://github.com/emissary-ingress/emissary/compare/v2.5.0...v2.5.1
 
 ### Emissary-ingress and Ambassador Edge Stack
 
@@ -94,6 +94,11 @@ it will be removed; but as it won't be user-visible this isn't considered a brea
   Kubernetes Nodes (regardless of whether the installation was migrated from 1.y or was a fresh 2.y
   install); fully supporting `v1` again should resolve these errors.
 
+## [2.5.0] November 03, 2022
+[2.5.0]: https://github.com/emissary-ingress/emissary/compare/v2.4.0...v2.5.0
+
+### Emissary-ingress and Ambassador Edge Stack
+
 - Bugfix: If a `Host` or `TLSContext` contained a hostname with a `:` then when using the 
   diagnostics endpoints `ambassador/v0/diagd` then an error would be thrown due to the parsing logic
   not  being able to handle the extra colon. This has been fixed and Emissary-ingress will not throw
diff --git a/docs/releaseNotes.yml b/docs/releaseNotes.yml
index 1c935bd74..a38f8fa9a 100644
--- a/docs/releaseNotes.yml
+++ b/docs/releaseNotes.yml
@@ -32,8 +32,8 @@
 
 changelog: https://github.com/emissary-ingress/emissary/blob/$branch$/CHANGELOG.md
 items:
-  - version: 2.5.0
-    date: '2022-11-03'
+  - version: 2.5.1
+    date: TBD
     notes:
       - title: Re-add support for getambassador.io/v1
         type: feature
@@ -46,6 +46,9 @@ items:
           installation was migrated from 1.y or was a fresh 2.y install); fully supporting
           <code>v1</code> again should resolve these errors.
 
+  - version: 2.5.0
+    date: '2022-11-03'
+    notes:
       - title: Diagnostics stats properly handles parsing envoy metrics with colons
         type: bugfix
         body: >-

[LanceEa]

looks like there still is a merge conflict

[LukeShu]

Rebased to resolve merge conflict in the changelog.

[AliceProxy]

This looks ok to me

[joshbranham]

What is the timetable for a 2.5.x release with this fix? Would love to resolve the constant spewing of this log if possible.

cacher.go:424] cacher (*unstructured.Unstructured): unexpected ListAndWatch error: failed to list getambassador.io/v1, Kind=Module: conversion webhook for getambassador.io/v2, Kind=Module failed: no kind "Module" is registered for version "getambassador.io/v1" in scheme "/go/cmd/apiext/main.go:50"; reinitializing...