dns gateway: create manifest for every filter#537
Conversation
Signed-off-by: wbpcode <wbphub@gmail.com>
Signed-off-by: wbpcode <wbphub@gmail.com>
nacx
left a comment
There was a problem hiding this comment.
Nice! I see this works fine for local extensions, but I think we want to have a consistent experience for remote extensions as well.
How can this be packaged in a way that allows usign it with --extension as well?
I guess the idea is to use the bundle concept and do something like:
boe run --extesion dns-gateway/dns-gateway-lookup.
If that's the idea:
- What about renaming to remove repetition (e.g.
dns-gateway/lookup)? - What needs to tb changed in the packaging so that
--extensionworks fine?
It's has been supported natively. Both the legacy: and the new: works. I personally thought the local is more interesting so I used that in the description. We needn't any additional work at packing. Same to future other extensions, our bundle design works perfect for both local and remote extensions if you create the manifests correctly. |
SGTM. Let me update the naming. |
|
The name updated and fixed an example configuration spelling error. |
Signed-off-by: wbpcode <wbphub@gmail.com>
|
Thanks! built-on-envoy/extensions/Dockerfile.rust Lines 29 to 31 in 91a860c |
|
@wbpcode I've pushed a commit to this PR, PTAL. It basically fixes the extension catalog generation and UI, as they currently skip bundle extensions, assuming they're not runnable. This caused the parent |
But the new I am prepare a PR to update the UI to show the bundle name beside the extension name. So, the users known it come from the same bundle. |
| | **Required** | No | | ||
| | **Allowed values** | `metadata-only`, `runnable` | | ||
|
|
||
| [↑ Top](#) | [↑ Extension Manifest](#reference) {/* if .SubProperties */} |
There was a problem hiding this comment.
I am a little doubt should we introduce this new concept. 🤔 In my opinion, all extension sets should be metadata only, include the dns-gateway.
We kept the previous dns-gateway name as alias of extension to keep backwards compatibility, to ensure it won't break users in production or CLI.
But in the UI/doc, we may needn't take the backwards compatibility into account? My thoughts is:
- For the the UI, just show new
lookupandcacheextensions (compare tosingle extension with different filter types, I guessmultiple extensions in same bundleis easier to understand and also match the reality). We can render the bundle name beside the extension name to distinguish them, so users could understand. - For the document, we should also generate a description page for extension set, and also link the extension set's description to the extensions?
Then users finally will see lookup|dns-gateway or dns-gateway|cache in the UI and document. And if you click, it will see a description to the bundle dns-gateway?
Let me know your thought 🙇
There was a problem hiding this comment.
That is fine for the UI.
Now, for the extension catalog on the website, what's your take? I kinda like having the "dns-gateway" bundle as an extension there. If we want to remove it, we also need the "lookup" and "register" child extensions to be shown in a way that they appear related. I mean, one extension alone won't help, so we should render teh catalog in a way that they show "close together", like showing them as "dns-gateway/lookup", for example (bundle_name/extension_name).
The caveat with this is that I don't want the composer extensions to show as composer/opa in the catalog, because the nature of this bundle is different than the nature of the dns-gateway bundle, and I wouldn't want to hardcode "composer" in there... This is why ended up adding a property, but I'm open to suggestions.
Another thing that I considered is just changing the existing extensionSet to a string, but that would break noe run for all existing clients once we push the updated manifests.
There was a problem hiding this comment.
like showing them as "dns-gateway/lookup", for example (bundle_name/extension_name).
Yeah. That's what I was thinking. And the composer extensions will also be annotated that they are part of composer bundle (what's the side effect for composer extensions if we show them belongs to same composer bundle?)
Basically I personally inclined to design the document/catalog based on the abstracted bundle/extensions and ignore the specific implementation of different bundle/extensions. And don't want to treat the composer as a special case.
Although the nature of composer and dns-gateway bundle are different, it's only something should be documented clearly at the bundle's description page.
From BoE market's perspective, they should be equivalent: a bundle of extensions, and in the future, we may have other bundles with other nature. We could use same solution to handle it. By this way, the code tree, the packaging, and configuration, the UI and catalog, all match the bundle/extensions concept unanimously.
Yes, the old one, and the two new ones, will appear. The 3 of them. |
Signed-off-by: Ignasi Barrera <ignasi@tetrate.io>
731f056 to
29cbd9f
Compare
|
Ok, I've replaced my last commit with this one to revert my changes and just show the bundle in the catalog and UI: 29cbd9f. With this, the caveat is that "parent" extensions won't appear in the catalog; only the child ones. WDYT?
|
Thanks so much. I personally prefer this direction. And we can optimize the UI based on feedback to improve experience if necessary. |



we even could do this:
Manifest is use unique identifier of an unique filter.