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.

Sign up for GitHub

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

Jump to bottom

Enable Helm chart recommendations, use 3-namespace deployment #117

Merged
merged 7 commits into from
Jan 8, 2025
Merged

Enable Helm chart recommendations, use 3-namespace deployment #117

merged 7 commits into from
Jan 8, 2025

Conversation

markgoddard
Copy link
Contributor

Setting the Helm chart recommendations enabled flag applies various
changes to the configuration, as described in [1]:

  • 2 namespace layout (spire-server & spire-system)
  • Namespace pod security standards
  • Priority class name
  • Prometheus
  • Strict mode
  • Security contexts

We also use a 3rd namespace for the Helm chart state and CRDs as
described in [2].

[1] https://spiffe.io/docs/latest/spire-helm-charts-hardened-about/recommendations/
[2] https://spiffe.io/docs/latest/spire-helm-charts-hardened-about/namespaces/

Fixes: #113
Fixes: #114

@markgoddard
Enable Helm chart recommendations, use 3-namespace deployment
212260f 
Setting the Helm chart recommendations enabled flag applies various
changes to the configuration, as described in [1]:

- 2 namespace layout (spire-server & spire-system)
- Namespace pod security standards
- Priority class name
- Prometheus
- Strict mode
- Security contexts

We also use a 3rd namespace for the Helm chart state and CRDs as
described in [2].

[1] https://spiffe.io/docs/latest/spire-helm-charts-hardened-about/recommendations/
[2] https://spiffe.io/docs/latest/spire-helm-charts-hardened-about/namespaces/

Fixes: #113
Fixes: #114
@markgoddard markgoddard self-assigned this Jan 3, 2025
@markgoddard
Update Helm value overrides for 3 namespace setup
f9dd2d5 
Arguably we could avoid customising these values and let Helm handle them.
@markgoddard
Add CA subject to Helm values
b58b48f 
This is required by Helm recommendations. We provide a default
CA subject, and this can be overridden using custom Helm values.
@markgoddard
Slim down Helm value overrides
74e5c5d 
Try to avoid hard coding Helm values where the chart can provide a
sensible default or compute a value from other values.

For example, some values depend on the namespaces used, such as the
server address, spire agent service account allow list, etc.
@markgoddard markgoddard marked this pull request as ready for review January 6, 2025 16:35
@markgoddard markgoddard enabled auto-merge January 6, 2025 16:40
@jsnctl jsnctl self-requested a review January 7, 2025 09:39
jsnctl
jsnctl reviewed Jan 7, 2025
View reviewed changes
Copy link
Contributor

@jsnctl jsnctl left a comment

Choose a reason for hiding this comment

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

Minor suggestion: but we could add a very lightweight integration test to verify the correct resources are in the correct new namespaces (i.e. a quick kubectl assert that the server is in the spire-server namespace, the CRs are available in spire-mgmt etc)

@markgoddard
Copy link
Contributor Author

Minor suggestion: but we could add a very lightweight integration test to verify the correct resources are in the correct new namespaces (i.e. a quick kubectl assert that the server is in the spire-server namespace, the CRs are available in spire-mgmt etc)

I was thinking that might be a good idea. This is definitely an area that a full fat programming language would be better suited to, but we could do something simple in bash.

@markgoddard
Add basic SPIRE resource check in integration tests
f00327e 
Currently performs a basic check that expected resources exist in the
expected namespaces.
@markgoddard
Merge main into issues/113
22f504e
@markgoddard
Update clusterspiffeid names in integration test for 3 namespace change
33fefea 
CRs are now created in the spire-mgmt namespace.
@markgoddard
Copy link
Contributor Author

Minor suggestion: but we could add a very lightweight integration test to verify the correct resources are in the correct new namespaces (i.e. a quick kubectl assert that the server is in the spire-server namespace, the CRs are available in spire-mgmt etc)

I was thinking that might be a good idea. This is definitely an area that a full fat programming language would be better suited to, but we could do something simple in bash.

Added some checks.

@jsnctl jsnctl self-requested a review January 8, 2025 10:17
@markgoddard markgoddard merged commit 12a0f6b into main Jan 8, 2025
6 checks passed
@markgoddard markgoddard deleted the issues/113 branch January 8, 2025 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nialdaly nialdaly nialdaly left review comments

@jsnctl jsnctl jsnctl approved these changes

@mattbates mattbates Awaiting requested review from mattbates

Assignees

@markgoddard markgoddard

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use a 3 namespace deployment Enable Helm chart recommendations
3 participants
@markgoddard @jsnctl @nialdaly