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

Add example for Role, RoleBinding and Secret to configure BGP passwords #1837

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

paketb0te
Copy link
Contributor

@paketb0te paketb0te commented Jan 14, 2025

Product Version(s):
Calico

Issue:
fixes projectcalico/calico#6893

Link to docs preview:
CLICK

SME review:

  • An SME has approved this change.

DOCS review:

  • A member of the docs team has approved this change.

Additional information:

The Docs (in the reource definition for BGP peers) show that BGP Passwords must be stored in a Secret and then be referenced via the KeyRef (name of the secret and the key within that secret).

BUT it does not mention that this requires additional privileges for the calico-node ServiceAccount to access this secret.

It obviously makes sense if you think about it, but IMO it would make a lot of sense to mention that somewhere - see also @caseydavenport comments in projectcalico/calico#6893

I think the included example might be a bit over the top, maybe there is a way to make it show only partially by default and expand on click or something?

I am not exactly familiar with mdx unfortunately.

Merge checklist:

  • Deploy preview inspected wherever changes were made
  • Build completed successfully
  • Test have passed

@paketb0te paketb0te requested a review from a team as a code owner January 14, 2025 14:07
Copy link

netlify bot commented Jan 14, 2025

Deploy Preview succeeded!

Built without sensitive environment variables

Name Link
🔨 Latest commit 8ed924f
🔍 Latest deploy log https://app.netlify.com/sites/tigera/deploys/67866f899141c90008781956
😎 Deploy Preview https://deploy-preview-1837--tigera.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 26 (🔴 down 1 from production)
Accessibility: 90 (no change from production)
Best Practices: 83 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

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

Copy link

netlify bot commented Jan 14, 2025

Deploy Preview for calico-docs-preview-next ready!

Name Link
🔨 Latest commit 8ed924f
🔍 Latest deploy log https://app.netlify.com/sites/calico-docs-preview-next/deploys/67866f890c88d90008bb7c0d
😎 Deploy Preview https://deploy-preview-1837--calico-docs-preview-next.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.

@ctauchen
Copy link
Collaborator

Thanks for this contribution, @paketb0te! I'll let @caseydavenport vet the technical side before I follow up with a docs review.

@ctauchen ctauchen added the community-contributor Author is from outside of Tigera label Jan 14, 2025
@ctauchen
Copy link
Collaborator

@caseydavenport I expect this new information would be universal. Am I right thinking we should eventually pick these changes to all products / versions?

kind: Role
metadata:
name: bgp-passwords-reader
namespace: kube-system
Copy link
Member

Choose a reason for hiding this comment

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

I think we want to default to calico-system (most new installs use the calico-system namespace).

Perhaps this is a use-case for Tabs for "Operator" and "Manifest" installs? We have those elsewhere in the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I don't mind which one is used as example :)

Maybe we could even make it extra-explicit by using something like $NAMESPACE_IN_WHICH_CALICO_IS_RUNNING

That would
a) make it non-copy-paste-able, and thus
b) ensure that people look up the namespace calico is actually running in.

I like these Tabs for the different installation methods because they help keep the docs a bit clearer, but I also think they are better used in the "explanatory" sections of the docs and could be confusing in the Resource Definition Reference.

Copy link
Member

Choose a reason for hiding this comment

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

I'd be happy with that approach :)

I'll leave it up to @ctauchen as to what the preferred style guideline would be in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contributor Author is from outside of Tigera
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation: Cluster Role lacks Permissions for BGP Peer Password secretKeyRef
3 participants