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

Updated Components topic for OIDC authentication #547

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

Conversation

n-boshnakov
Copy link
Contributor

What this PR does / why we need it:
This PR updates screenshot and text in the Logging into Plutono section of the Components topic with details on logging in with OIDC authentication. It also adds information on programmatic access.

Which issue(s) this PR fixes:
Fixes #545

Special notes for your reviewer:

Release note:

NONE

@n-boshnakov n-boshnakov requested a review from a team as a code owner November 5, 2024 14:10
@gardener-robot gardener-robot added needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels Nov 5, 2024
rickardsjp
rickardsjp previously approved these changes Nov 5, 2024
Copy link
Member

@rickardsjp rickardsjp left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review labels Nov 5, 2024
@donistz donistz requested a review from nickytd November 5, 2024 15:00

![access-plutono](./images/access-plutono.png)

Programmatic access is still possible via the non-OIDC ingress using the credentials from the `<clustername>.monitoring` secret. It contains the HTTP basic auth credentials in base64-encoded form, as well as the Plutono ingress URL. The Prometheus URL can be derived from the Plutono URL by replacing the `gu` prefix with `p`.
Copy link

Choose a reason for hiding this comment

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

programmatic access although possible is not a publicly supported scenario. We still shall explain that user name password access is possible. The latter values are indeed stored in <clustername>.monitoring secret and the url as present as an annotation on the same secret. But again please do not market this as programmatic access. Today we don't support that scenario.

Copy link
Member

Choose a reason for hiding this comment

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

@nickytd do we have an issue or even a timeline for programmatic access? If we don't offer an alternative solution to users who need machine-to-machine access to e.g. Prometheus, we're forcing them to go with the deprecated option. I don't expect them to be happy with that.

Copy link

Choose a reason for hiding this comment

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

I am not aware of such plans. That is why we keep the current state preserving the use case.

Copy link
Member

Choose a reason for hiding this comment

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

In that case I disagree with marking it as deprecated. It's a feature we're expected to provide, and without a replacement it should not be considered deprecated IMO.

Copy link

Choose a reason for hiding this comment

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

I will agree with you If you point me to the scenario description of programatic access service offering. Otherwise I am against this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I will agree with you If you point me to the scenario description of programatic access service offering.

Was there a GEP discussing the move from basic auth to OIDC and dropping the support for federating metrics? I'm not entirely sure, where/if this kind of documentation exists at all for the observability area 🤔 @nickytd maybe you can help me out? Thanks!

Anyways, from the perspective of a Gardener consumer/user, basic auth access as an implementation of programmatic access is essential to federate metrics. If we are indeed deprecating it, a proper replacement needs to be in place before.

Copy link
Member

Choose a reason for hiding this comment

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

To add to Hendrik's point: The ability to federate control plane metrics using the basic auth credentials has been an option available to users since before the OIDC ingresses were introduced.

Copy link

@nickytd nickytd Nov 6, 2024

Choose a reason for hiding this comment

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

I see. Perhaps there is some historical service offering to which I am not aware. In this case, it is better, the service (scenario) owner of the programatic access to define the correct wording, reflecting the desired service offering.
As for the OIDC it is an addition to the current auth & auth schemes and not a replacement.

JordanJordanov
JordanJordanov previously approved these changes Nov 6, 2024
Copy link
Member

@JordanJordanov JordanJordanov left a comment

Choose a reason for hiding this comment

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

/lgtm

Change due to discussion on `programmatic access`
@n-boshnakov
Copy link
Contributor Author

@nickytd @rickardsjp I have omitted the mention of "programmatic access" in the topic. Is there a need for further updates, or can the PR be merged as is?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reviewed/lgtm Has approval for merging size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Components - OIDC access is not explained for the SAP Gardener service
6 participants