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

Add support for enabling the spire-agent admin socket #234

Merged
merged 15 commits into from
Feb 9, 2024
Merged

Add support for enabling the spire-agent admin socket #234

merged 15 commits into from
Feb 9, 2024

Conversation

kfox1111
Copy link
Collaborator

@kfox1111 kfox1111 commented Feb 4, 2024

Fixes: #224

@kfox1111
Add support for enabling the spire-agent admin socket
7cae9ed 
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
@kfox1111
Fix typo
d7c5e4a 
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
@kfox1111
Update permissions
c8c38a6 
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
@kfox1111
Update permissions
0410224 
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
@kfox1111
Fix template
db9f350 
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
@kfox1111
Update upstream agent too
082db61 
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
@kfox1111 kfox1111 force-pushed the spire-agent-admin-socket branch from ac982ce to 082db61 Compare February 4, 2024 23:15
@faisal-memon
Copy link
Collaborator

@edwbuck Can you take a look and see if this works for you?

@kfox1111
Merge branch 'main' into spire-agent-admin-socket
643f16d 
Signed-off-by: kfox1111 <Kevin.Fox@pnnl.gov>
@faisal-memon faisal-memon added this to the 0.18.0 milestone Feb 5, 2024
edwbuck
edwbuck reviewed Feb 8, 2024
View reviewed changes
Copy link
Collaborator

@edwbuck edwbuck left a comment

Choose a reason for hiding this comment

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

I don't see any checks to ensure that the agent admin socket is not in the same directory (or a sub directory) of the agent socket.

Considering that when someone configures this with the agent admin socket in the same directory or below the agent socket directory the agent won't launch, I think we should handle this misconfiguration in the helm template (rather than let them publish the wrong values and watch their agents enter a crash loop).

charts/spire/charts/spire-agent/README.md Outdated Show resolved Hide resolved
charts/spire/charts/spire-agent/templates/configmap.yaml Show resolved Hide resolved
charts/spire/charts/spire-agent/README.md Show resolved Hide resolved
charts/spire/charts/spire-agent/templates/daemonset.yaml Show resolved Hide resolved
charts/spire/charts/spire-agent/README.md Outdated Show resolved Hide resolved
charts/spire/charts/spire-agent/values.yaml Show resolved Hide resolved
@faisal-memon faisal-memon mentioned this pull request Feb 8, 2024
Open
@faisal-memon
Copy link
Collaborator

I don't see any checks to ensure that the agent admin socket is not in the same directory (or a sub directory) of the agent socket.

@kfox1111 I think this is a good one to include. Do you have the bandwidth to address? If not I can push a commit that does this check.

@kfox1111
Incorperate feedback
a63b44e 
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
@kfox1111
Copy link
Collaborator Author

kfox1111 commented Feb 9, 2024

I don't see any checks to ensure that the agent admin socket is not in the same directory (or a sub directory) of the agent socket.

@kfox1111 I think this is a good one to include. Do you have the bandwidth to address? If not I can push a commit that does this check.

I have added this test.

edwbuck
edwbuck approved these changes Feb 9, 2024
View reviewed changes
Copy link
Collaborator

@edwbuck edwbuck left a comment

Choose a reason for hiding this comment

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

approved

@faisal-memon faisal-memon merged commit a2e5a4e into main Feb 9, 2024
28 checks passed
@faisal-memon faisal-memon deleted the spire-agent-admin-socket branch February 9, 2024 20:44
faisal-memon added a commit that referenced this pull request Mar 4, 2024
@faisal-memon
Bump spire Helm Chart version from 0.17.2 to 0.18.0
fd2f92e 
* 5849ea2 add pod labels to spire server values (#271)
* f512b06 Configurable daemonsets updateStrategy (#212)
* a539065 Add direct tpm support for spire-agent (#216)
* fcd0c11 Add direct tpm support for spire-server (#211)
* c570174 Enable CA settings via global (#268)
* ac83694 Initial SPIRE 1.9.0 support (#262)
* ddb4eff Bump test chart dependencies (#263)
* bfbafbc Fix OpenShift Federation Ingress bug (#260)
* a0baace Upgrade to spire-controller-manager 0.4.3 (#258)
* 1446f7e Add support for specifying agent authorized_delegates (#255)
* 0b6cd88 Add support for specifying server admin_ids (#254)
* 07a1c39 Add global override for kubectl tag (#251)
* b82a84d Bump test chart dependencies (#252)
* 7a1e731 Bump test chart dependencies (#246)
* a706063 make audit_log_enabled configurable (#241)
* 34a39cb Added emptyDir volume to spire-agent SCC (#243)
* a2e5a4e Add support for enabling the spire-agent admin socket (#234)
* febdcbf Fix whitespace in spire-agent daemonset

Signed-off-by: Faisal Memon <fymemon@yahoo.com>
faisal-memon added a commit that referenced this pull request Mar 4, 2024
@faisal-memon
Bump spire Helm Chart version from 0.17.2 to 0.18.0
260501b 
* beda725 Add pod labels to the SPIRE agent (#273)
* 077f152 Bump test chart dependencies (#272)
* 5849ea2 add pod labels to spire server values (#271)
* f512b06 Configurable daemonsets updateStrategy (#212)
* a539065 Add direct tpm support for spire-agent (#216)
* fcd0c11 Add direct tpm support for spire-server (#211)
* c570174 Enable CA settings via global (#268)
* ac83694 Initial SPIRE 1.9.0 support (#262)
* ddb4eff Bump test chart dependencies (#263)
* bfbafbc Fix OpenShift Federation Ingress bug (#260)
* a0baace Upgrade to spire-controller-manager 0.4.3 (#258)
* 1446f7e Add support for specifying agent authorized_delegates (#255)
* 0b6cd88 Add support for specifying server admin_ids (#254)
* 07a1c39 Add global override for kubectl tag (#251)
* b82a84d Bump test chart dependencies (#252)
* 7a1e731 Bump test chart dependencies (#246)
* a706063 make audit_log_enabled configurable (#241)
* 34a39cb Added emptyDir volume to spire-agent SCC (#243)
* a2e5a4e Add support for enabling the spire-agent admin socket (#234)
* febdcbf Fix whitespace in spire-agent daemonset

Signed-off-by: Faisal Memon <fymemon@yahoo.com>
@faisal-memon faisal-memon mentioned this pull request Mar 4, 2024
Merged
faisal-memon added a commit that referenced this pull request Mar 4, 2024
@faisal-memon
Bump spire Helm Chart version from 0.17.2 to 0.18.0 (#274)
cf2e213 
* beda725 Add pod labels to the SPIRE agent (#273)
* 077f152 Bump test chart dependencies (#272)
* 5849ea2 add pod labels to spire server values (#271)
* f512b06 Configurable daemonsets updateStrategy (#212)
* a539065 Add direct tpm support for spire-agent (#216)
* fcd0c11 Add direct tpm support for spire-server (#211)
* c570174 Enable CA settings via global (#268)
* ac83694 Initial SPIRE 1.9.0 support (#262)
* ddb4eff Bump test chart dependencies (#263)
* bfbafbc Fix OpenShift Federation Ingress bug (#260)
* a0baace Upgrade to spire-controller-manager 0.4.3 (#258)
* 1446f7e Add support for specifying agent authorized_delegates (#255)
* 0b6cd88 Add support for specifying server admin_ids (#254)
* 07a1c39 Add global override for kubectl tag (#251)
* b82a84d Bump test chart dependencies (#252)
* 7a1e731 Bump test chart dependencies (#246)
* a706063 make audit_log_enabled configurable (#241)
* 34a39cb Added emptyDir volume to spire-agent SCC (#243)
* a2e5a4e Add support for enabling the spire-agent admin socket (#234)
* febdcbf Fix whitespace in spire-agent daemonset

Signed-off-by: Faisal Memon <fymemon@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amoore877 amoore877 amoore877 left review comments

@marcofranssen marcofranssen marcofranssen left review comments

@edwbuck edwbuck edwbuck approved these changes

@faisal-memon faisal-memon faisal-memon approved these changes

@dfeldman dfeldman Awaiting requested review from dfeldman

@mrsabath mrsabath Awaiting requested review from mrsabath mrsabath is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.18.0
Development

Successfully merging this pull request may close these issues.

Ensure that the agent admin socket can be enabled
5 participants
@kfox1111 @faisal-memon @marcofranssen @edwbuck @amoore877