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

ENG-4153 | Document setting up GKE Workload Identity/EKS Pod Identity #200

Conversation

janekbaraniewski
Copy link
Contributor

@janekbaraniewski janekbaraniewski commented Jul 17, 2024

  • We want to move this from the platform docs to the vCluster docs as most people will search for this in the vcluster docs. We also want to group both pages under Pod Identity and then have EKS and GKE.
  • EKS: Integrations -> Identity -> EKS
  • GKE: Integrations -> Identify -> GKE
  • Since you need the platform, these pages should be labeled as pro, so it shows up in the side bar: Example front matter that you'll need: https://github.com/loft-sh/vcluster-docs/blob/main/vcluster/configure/vcluster-yaml/experimental/isolated-control-plane.mdx?plain=1#L5
  • Since it will be under vCluster now instead of the platform, we should remove the vCluster Platform running as a pre-req and instead it becomes one of the steps of the guide. In that step, it should refer to the install vCluster Platform docs.
  • Can the formats of the two guide match and follow the EKS style for the heading? So don't have headings that say Step 1 but instead 1.
  • For the steps that are deploying the vcluster, can you either use the partial from the basics page as you can deploy vCluster however you want? https://github.com/loft-sh/vcluster-docs/blob/main/vcluster/deploy/basics.mdx?plain=1#L180
  • Include Terraform examples and vcluster-terraform-modules usage
  • Test all possible paths:
    • GKE using bash + cli's
    • GKE using terraform
    • EKS using bash + cli's
    • EKS using terraform

Preview: https://deploy-preview-200--vcluster-docs-site.netlify.app/docs/platform/integrations/gke-workload-identity

Copy link

netlify bot commented Jul 17, 2024

Deploy Preview for vcluster-docs-site ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 38b1d80
🔍 Latest deploy log https://app.netlify.com/sites/vcluster-docs-site/deploys/66b0cee7bed9170008aab337
😎 Deploy Preview https://deploy-preview-200--vcluster-docs-site.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@janekbaraniewski janekbaraniewski marked this pull request as ready for review July 22, 2024 14:34
@heiko-braun
Copy link
Contributor

Thanks @janekbaraniewski this escaped my attention. I will take a look at it tomorrow

@heiko-braun heiko-braun self-assigned this Jul 24, 2024
@heiko-braun
Copy link
Contributor

@janekbaraniewski why did we drop the terraform module from the docs?

zerbitx
zerbitx previously approved these changes Jul 30, 2024
Copy link
Contributor

@zerbitx zerbitx 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
Contributor

@deniseschannon deniseschannon left a comment

Choose a reason for hiding this comment

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

  1. We want to move this from the platform docs to the vCluster docs as most people will search for this in the vcluster docs. We also want to group both pages under Pod Identity and then have EKS and GKE.
  2. EKS: Integrations -> Identity -> EKS
  3. GKE: Integrations -> Identify -> GKE
  4. Since you need the platform, these pages should be labeled as pro, so it shows up in the side bar: Example front matter that you'll need: https://github.com/loft-sh/vcluster-docs/blob/main/vcluster/configure/vcluster-yaml/experimental/isolated-control-plane.mdx?plain=1#L5
  5. Since it will be under vCluster now instead of the platform, we should remove the vCluster Platform running as a pre-req and instead it becomes one of the steps of the guide. In that step, it should refer to the install vCluster Platform docs.
  6. Can the formats of the two guide match and follow the EKS style for the heading? So don't have headings that say Step 1 but instead 1.
  7. For the steps that are deploying the vcluster, can you either use the partial from the basics page as you can deploy vCluster however you want? https://github.com/loft-sh/vcluster-docs/blob/main/vcluster/deploy/basics.mdx?plain=1#L180

@janekbaraniewski janekbaraniewski force-pushed the ENG-4153/add-docs-on-workload-identity-pod-identity branch from 60dc669 to 7a0cc70 Compare July 30, 2024 19:06
@deniseschannon
Copy link
Contributor

deniseschannon commented Aug 2, 2024

Can you run it through spell check real quick and maybe grammarly or a grammar check? I found a spelling error, but it might be easier to have you run it through spelling rather than me comment on each one.

Also replace hosting cluster with the host cluster in any references

@janekbaraniewski janekbaraniewski force-pushed the ENG-4153/add-docs-on-workload-identity-pod-identity branch from a443321 to cf916ba Compare August 5, 2024 09:06
@janekbaraniewski janekbaraniewski force-pushed the ENG-4153/add-docs-on-workload-identity-pod-identity branch from cf916ba to 38b1d80 Compare August 5, 2024 13:08
Copy link
Contributor

@deniseschannon deniseschannon left a comment

Choose a reason for hiding this comment

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

Nit: if we declare the env vars with all caps, can we reference them with all caps as well through out the docs?

@deniseschannon deniseschannon merged commit dd85c47 into loft-sh:main Aug 6, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants