Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP first pass for ambient api page #15740

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

linsun
Copy link
Member

@linsun linsun commented Sep 27, 2024

Part of istio/istio#52698

Reviewers

  • Ambient
  • [ x] Docs
  • Installation
  • Networking
  • Performance and Scalability
  • Extensions and Telemetry
  • Security
  • Test and Release
  • User Experience
  • Developer Infrastructure
  • Localization/Translation

@linsun linsun requested a review from a team as a code owner September 27, 2024 15:41
@istio-testing istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 27, 2024
@linsun linsun changed the title first pass for ambient api page WIP first pass for ambient api page Sep 27, 2024
@istio-testing istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Sep 27, 2024
@linsun
Copy link
Member Author

linsun commented Sep 30, 2024

Anything else should be added to this page @howardjohn @ilrudie @keithmattix @louiscryan ?


| Name | Status |
| --- | --- |
| [`HTTPRoute`](https://gateway-api.sigs.k8s.io/guides/http-routing/) | Beta |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTTPRoute is GA and so is Istio's support for it generally. If this is documentation for Ambient GA, shouldn't this status be GA as well?

| Name | Status |
| ------------------------------- | -------------------------- |
| `gateway.istio.io/service-account` | Alpha |
| `ambient.istio.io/waypoint-inbound-binding` | Alpha |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually have no idea what this does

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's undocumented/WIP/experimental AFAIK: istio/istio#50235

I'd be in favor of simply not mentioning it in this doc at all at this time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah I'd leave this completely undocumented; I don't even know if I see sandwich on istio.io anywhere


| Name | Status |
| ------------------------------- | -------------------------- |
| `istio.io/dataplane-mode` | Beta |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to comment below: if this is GA documentation, these labels should be GA

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would drop this and annotations, its already documented on a dedicated page (istio/api#3307 is important though)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intent is to have everything in one page while linking to the official API reference page, so users don't need to go through 100 labels/annotations to identify which ones (in fact only a few of them) are ambient related.


## Istio resources

Below are the Istio resources you can use to build networking or security or other policies for your services in the ambient mode:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a sentence here about how the project recommends using Gateway API as the default and only using these resources when you need to. And maybe sort the resources in order of safety just for subliminal messaging (e.g. AuthZ policy first, then ServiceEntry, then DestinationRule, etc.).

| Name | Status |
| --- | --- |
| [`VirtualService`](https://gateway-api.sigs.k8s.io/guides/http-routing/) | Alpha |
| [`DestinationRule`](https://gateway-api.sigs.k8s.io/guides/tls) | Beta |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 3 (DR, SE, and AP) should be GA

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the links to Istio APIs shouldn't go to the Gateway API site?

Copy link
Contributor

@bleggett bleggett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this designed to document

  • labels/annos that are stable/GA (and thus likely already documented elsewhere)?
  • labels/annos that are not stable/GA (and thus likely not already documented elsewhere)?
  • both?

Perhaps we simply should not document unstable annos/labels. If they're important enough to surface to end-users, that's maybe a good signal they should be stabilized and documented.

(if we just want an inventory of all the unstable candidate annos/labels for our own purposes, that maybe can just be in a dev wiki and not user-facing docs).

Otherwise, mentioning them here probably just helps codify them as "supported" when they may in fact be entirely unused/associated with WIP features, and we may not want that? In general I feel like documenting alpha anything in user-facing docs is a bad habit we should always avoid.

@linsun
Copy link
Member Author

linsun commented Oct 1, 2024

Is this designed to document

* labels/annos that _are_ stable/GA (and thus likely already documented elsewhere)?

* labels/annos that are _not_ stable/GA (and thus likely not already documented elsewhere)?

* both?

Perhaps we simply should not document unstable annos/labels. If they're important enough to surface to end-users, that's maybe a good signal they should be stabilized and documented.

(if we just want an inventory of all the unstable candidate annos/labels for our own purposes, that maybe can just be in a dev wiki and not user-facing docs).

Otherwise, mentioning them here probably just helps codify them as "supported" when they may in fact be entirely unused/associated with WIP features, and we may not want that? In general I feel like documenting alpha anything in user-facing docs is a bad habit we should always avoid.

The intention is to have 1 single page on ambient related APIs so user can easily understand what APIs are for ambient and their feature status. Yes, we probably have the labels/annotations documented elsewhere but I felt would be good to have everything in one page. Some of them could be alpha as that is the current state so users are well informed when they choose to use an alpha API.

@bleggett
Copy link
Contributor

bleggett commented Oct 1, 2024

Some of them could be alpha as that is the current state so users are well informed when they choose to use an alpha API.

I don't think any alpha annotations or labels are required for ambient GA usage, correct?

I think, based on the actual feature definitions we always have had which are pretty clear that alpha means "general public should not use", we simply should never document any labels or annotations that are not Beta or higher.

If this causes a problem, or someone needs that label or annotation, the relevant non-beta labels or annotations should be promoted to beta or better, and documented at that time.

That way we can point at this doc and simply say "if it's not listed here, it's Alpha or worse".

(the ship has probably sailed on doing this for Istio custom resource APIs, but for labels and annotations specifically, I think it makes a lot of sense)

@keithmattix
Copy link
Contributor

keithmattix commented Oct 1, 2024

I don't think any alpha annotations or labels are required for ambient GA usage, correct?

Put another way, if we require alpha labels/annotations for ambient GA usage, then Ambient's not at GA quality.

@bleggett
Copy link
Contributor

bleggett commented Oct 1, 2024

Yeah and since

already exist as "unstable" developer documentation for Alpha-or-below labels/annotations, I don't see the need to mention Alpha-or-below labels/annotations in the user facing docs at all - that just confuses people, I feel. Better to only list the stable ones and call it a day.

Is it stable? --> do you see it referenced in this user-facing doc?

  • yes? then it's stable (Beta or better).
  • no? then it's not (except for EnvoyFilter) and you shouldn't use it.

I don't think we should write istio.io documentation for anything Alpha-or-below, as a general rule (EnvoyFilter probably being the really major exception).

If we want to refer to them in user-facing docs they should be Beta or higher, which seems like a perfectly reasonable forcing function for stabilizing things (and for not writing docs around features that haven't stabilized, thereby creating the expectation we will support them as-is).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ambient do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. kind/docs size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants