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

Controller manager integration #6

Merged

Conversation

alexandrealvino
Copy link
Contributor

@alexandrealvino alexandrealvino commented May 25, 2022

Affected functionality
Mithril automatic workload registration

Description of change
Replaces k8s automatic registrar with the new SPIRE Controller Manager

Which Jira task this PR is related to
Jira task

Signed-off-by: Alexandre Alvino [email protected]

Signed-off-by: Alexandre Alvino <[email protected]>
Signed-off-by: Alexandre Alvino <[email protected]>
Signed-off-by: Alexandre Alvino <[email protected]>
Signed-off-by: Alexandre Alvino <[email protected]>
Signed-off-by: Alexandre Alvino <[email protected]>
Signed-off-by: Alexandre Alvino <[email protected]>
@hpe-prow
Copy link

hpe-prow bot commented May 25, 2022

Hi @alexandrealvino. Thanks for your PR.

I'm waiting for a HewlettPackard member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link
Contributor

@mgbcaio mgbcaio left a comment

Choose a reason for hiding this comment

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

Nice job, Alex. Left some comments. Let me know what you think. 💯

POC/spire/spiffe-ids.yaml Outdated Show resolved Hide resolved
usecases/federation/bookinfo/deploy-bookinfo.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@n-th n-th left a comment

Choose a reason for hiding this comment

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

🍍

@@ -0,0 +1,299 @@
apiVersion: v1
Copy link
Member

Choose a reason for hiding this comment

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

is this file being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, only for me in my local env haha. I will remove this leftover, thanks!

Signed-off-by: Alexandre Alvino <[email protected]>
metadata:
name: ingress
spec:
spiffeIDTemplate: spiffe://example.org/ingress/{{ .PodMeta.Name }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why doesn't the ingress use spiffeIDTemplate with /ns/xxx/sa/xxx?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While I was testing the integration I was able to identify that the spiffeid pattern /ns/xxx/sa/xxx is only necessary for workloads, thats why I used this configuration to emphasize this behavior.

Copy link
Contributor

@maxlambrecht maxlambrecht left a comment

Choose a reason for hiding this comment

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

Nicely done! 🍪

Copy link
Member

@mchurichi mchurichi left a comment

Choose a reason for hiding this comment

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

🍨

@hpe-prow
Copy link

hpe-prow bot commented Jun 3, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: alexandrealvino, maxlambrecht, mchurichi, n-th

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@alexandrealvino alexandrealvino merged commit 8e64573 into HewlettPackard:main Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants