Skip to content

docs: Promote user-info-fetcher docs to navgation #557

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

Merged
merged 4 commits into from
Jul 23, 2024

Conversation

sbernauer
Copy link
Member

Description

Given that we are using it extensively already (e.g. in the e2e-security demo) and me handing out the link to multiple customers, I would like to promote https://docs.stackable.tech/home/nightly/opa/usage-guide/user-info-fetcher.html to include it in the navigation

Definition of Done Checklist

  • Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
  • Please make sure all these things are done and tick the boxes
# Author
- [ ] Changes are OpenShift compatible
- [ ] CRD changes approved
- [ ] CRD documentation for all fields, following the [style guide](https://docs.stackable.tech/home/nightly/contributor/docs-style-guide).
- [ ] Helm chart can be installed and deployed operator works
- [ ] Integration tests passed (for non trivial changes)
- [ ] Changes need to be "offline" compatible
# Reviewer
- [ ] Code contains useful comments
- [ ] Code contains useful logging statements
- [ ] (Integration-)Test cases added
- [ ] Documentation added or updated. Follows the [style guide](https://docs.stackable.tech/home/nightly/contributor/docs-style-guide).
- [ ] Changelog updated
- [ ] Cargo.toml only contains references to git tags (not specific commits or branches)
# Acceptance
- [ ] Feature Tracker has been updated
- [ ] Proper release label has been added
- [ ] [Roadmap](https://github.com/orgs/stackabletech/projects/25/views/1) has been updated

@sbernauer sbernauer requested review from nightkr, NickLarsenNZ and a team April 26, 2024 06:19
@sbernauer sbernauer self-assigned this Apr 26, 2024
Maleware
Maleware previously approved these changes Apr 26, 2024
Copy link
Member

@Maleware Maleware left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you :)

@sbernauer
Copy link
Member Author

Giving @nightkr and @NickLarsenNZ a chance to comment...

@nightkr
Copy link
Member

nightkr commented Apr 26, 2024

Have we addressed the client API side yet? We shouldn't have been shipping it to demos or customers before that, and the linked docs definitely don't.

@sbernauer
Copy link
Member Author

Are you talking about the API described in https://docs.stackable.tech/home/nightly/opa/usage-guide/user-info-fetcher.html#_user_info_fetcher_api?
Currently you can send an username or id and get all infos we can collect back. What exactly do you want to address, would you like to change the API?

@nightkr
Copy link
Member

nightkr commented Apr 29, 2024

The UIF provides the HTTP API, but we should wrap that in an OPA rule we provide (and can update), rather than have everyone hard-code the HTTP stuff themselves.

@fhennig
Copy link
Contributor

fhennig commented Apr 30, 2024

I don't think we have that yet. I agree that it would be good to have. Could we simply have a uif rego package that we deploy when the UIF is enabled? and users can import the package in their own code?

I think we can still do this 'promotion' here and do a follow-up PR to adress this. it's all in nightly anyways, but we need to prioritise the follow-up then

edit: I made a ticket for this: #558

@sbernauer
Copy link
Member Author

Yeah, I can see the point of providing a regorule in addition to the HTTP API. The question is if we should block the docs on the API until the rego rules are there 🤔

@nightkr
Copy link
Member

nightkr commented May 2, 2024

I don't think we have that yet. I agree that it would be good to have. Could we simply have a uif rego package that we deploy when the UIF is enabled? and users can import the package in their own code?

Yes, basically this.

I think we can still do this 'promotion' here and do a follow-up PR to adress this. it's all in nightly anyways, but we need to prioritise the follow-up then.

I'd rather avoid keeping main in an unreleasable state.. but especially when even temporary nightly changes end up on https://docs.stackable.tech/home/nightly/index.html immediately (almost) and cause more confusion.

Yeah, I can see the point of providing a regorule in addition to the HTTP API. The question is if we should block the docs on the API until the rego rules are there 🤔

We should, because the current HTTP API is not public or stable. The uif package is the stable API surface, the HTTP API is just an implementation detail for how to get there.

@fhennig
Copy link
Contributor

fhennig commented May 2, 2024

Okay I agree with @nightkr, so maybe we can put #558 on the board either to be refined or ready for development, and then block this PR on the other ticket? Or we shelf the whole thing for now. paging @lfrancke

fhennig
fhennig previously requested changes May 2, 2024
Copy link
Contributor

@fhennig fhennig left a comment

Choose a reason for hiding this comment

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

blocked by #558

@sbernauer
Copy link
Member Author

I think the change is trivial and I would close this PR until everyone thinks we are ready to include the docs in the navigation

@sbernauer
Copy link
Member Author

Reopening according to the decision in the daily, as we where not able to ship #558 in time for the 23.07 release

@Techassi Techassi self-requested a review July 23, 2024 07:56
NickLarsenNZ
NickLarsenNZ previously approved these changes Jul 23, 2024
Copy link
Member

@NickLarsenNZ NickLarsenNZ left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@Techassi Techassi left a comment

Choose a reason for hiding this comment

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

Some suggestions.

@sbernauer sbernauer requested a review from Techassi July 23, 2024 08:05
@Techassi Techassi added this pull request to the merge queue Jul 23, 2024
Merged via the queue into main with commit c9684b9 Jul 23, 2024
31 checks passed
@Techassi Techassi deleted the chore/promote-uif-docs branch July 23, 2024 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants