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

feat: rewrite Operate API auth guide #4138

Merged
merged 9 commits into from
Aug 26, 2024

Conversation

pepopowitz
Copy link
Collaborator

Description

Part of #4117.

Rewrites the Operate API Auth guide to align with the specs defined in #4117.

Once approved, I will backport to the current version of the docs also.

SaaS

image

Self-Managed

image

Using a token/etc

image

When should this change go live?

  • This is a bug fix, security concern, or something that needs urgent release support.
  • This is already available but undocumented and should be released within a week.
  • This on a specific schedule and the assignee will coordinate a release with the DevEx team. (apply hold label or convert to draft PR)
  • This is part of a scheduled alpha or minor. (apply alpha or minor label)
  • There is no urgency with this change and can be released at any time.

PR Checklist

  • My changes are for an already released minor and are in /versioned_docs directory.
  • My changes are for the next minor and are in /docs directory (aka /next/).

@pepopowitz pepopowitz added component:docs Documentation improvements, including new or updated content theme:api-streamline Issues related to the theme of streamlining APIs labels Aug 9, 2024
@pepopowitz pepopowitz requested review from akeller and a team August 9, 2024 22:01
@pepopowitz pepopowitz self-assigned this Aug 9, 2024
Copy link
Contributor

github-actions bot commented Aug 9, 2024

👋 🤖 ✅ Looks like the changes were ported across versions, nice job! 🎉

You can read more about the versioning within our docs in our documentation guidelines.

@akeller
Copy link
Member

akeller commented Aug 12, 2024

@pepopowitz can you bring someone in from the Operate team via #ask-operate to review this PR as well?


### Authentication via cookie
## Authentication via cookie (Self-Managed only)
Copy link
Member

Choose a reason for hiding this comment

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

❓ This is the only API that has docs about cookie auth, which feels suspicious.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My assumption is that this exists for a reason, but I've started a conversation at https://camunda.slack.com/archives/C9B5270DA/p1724352947450009 to hopefully get more clarity.

@akeller akeller requested a review from a team August 15, 2024 17:05
@mesellings mesellings self-requested a review August 16, 2024 07:32

1. [Create client credentials](/guides/setup-client-connection-credentials.md) in the **Clusters > Cluster name > API** tab of [Camunda Console](https://console.camunda.io/).
2. Add permissions to this client for **Operate**.
3. Upon creating the client, capture the following values required to generate a token:
Copy link
Contributor

Choose a reason for hiding this comment

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

NBC: "When" instead of "upon"?

Copy link
Collaborator Author

@pepopowitz pepopowitz Aug 22, 2024

Choose a reason for hiding this comment

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

I actually changed this from when to upon, because it felt wrong. You see the values as soon as you're done creating the client, not while you're doing it. I didn't like "after," because it implies that you can go back and get the values later (which you can't do).

Is there a different word to use here?

Copy link
Contributor

Choose a reason for hiding this comment

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

"Once"[you have created...] implies straight after
"After" has less emphasis on it being straight after
"When" implies during the process
I guess then it should be after or once, but once is a hurdle in translation as it can also mean numerically. So perhaps we can use "Once" for now, and if translation becomes a thing, we know to revisit this phrasing?

Copy link
Contributor

Choose a reason for hiding this comment

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

So it could be: Once you have created the client, capture...

| Audience | | `operate.camunda.io` |
| Operate REST Address | `CAMUNDA_OPERATE_BASE_URL` | - |
<!-- this comment convinces the markdown processor to still treat the table as a table, but without adding surrounding paragraphs. 🤷 -->
:::tip
Copy link
Contributor

Choose a reason for hiding this comment

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

NBC: Should this be a caution instead of a tip as it is quite important as they cannot get the secret again if they don't copy it now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't feel strongly either way about this; if you do, let me know and I'll adjust.

<TabItem value='saas'>

:::tip
The URL of the Operate REST API, represented below by the `${CAMUNDA_OPERATE_BASE_URL}` variable, can be captured when creating an API client. It can also be constructed as `https://${REGION}.operate.camunda.io/${CLUSTER_ID}`.
Copy link
Contributor

@mesellings mesellings Aug 16, 2024

Choose a reason for hiding this comment

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

NBC: Change passive voice > active voice

You can capture the URL of the Operate REST API when creating an API client. This is represented below....variable. It can...

An easy way to see if you are using passive voice is to see if you can insert "by zombies" and it still makes grammatical sense. For example:

can be captured (by zombies) = passive voice

Copy link
Contributor

@mesellings mesellings left a comment

Choose a reason for hiding this comment

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

Great job @pepopowitz looks really good - concise and clean!

I've left some non-blocking suggestions that you might feel helps, at your discretion if you think they are worth acting on, as some of them are for more of a wider discussion (e.g. using gerunds in headings).

I haven't explicitly approved as I believe an Operate team member needs to perform a review as well?

Copy link
Contributor

@christinaausley christinaausley left a comment

Choose a reason for hiding this comment

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

@mesellings Has left a detailed review, largely for active/passive voice, but beyond that, I think this looks great! 👍

@pepopowitz
Copy link
Collaborator Author

This PR is ready for a final review, with all required feedback addressed, and backported to v8.5.

Copy link
Member

@akeller akeller left a comment

Choose a reason for hiding this comment

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

The cleanup here looks fab! Thank you for opening up the conversation about cookie auth. Since that is an ongoing discussion, I wouldn't consider that blocking for this PR.

@pepopowitz pepopowitz merged commit e3e9c9b into main Aug 26, 2024
10 checks passed
@pepopowitz pepopowitz deleted the pepopowitz/4117-unify-api-auth-operate branch August 26, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:docs Documentation improvements, including new or updated content theme:api-streamline Issues related to the theme of streamlining APIs
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants