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

fix: shutdown-manager not respecting security context of container spec #4938

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Dean-Coakley
Copy link
Contributor

@Dean-Coakley Dean-Coakley commented Dec 16, 2024

What type of PR is this?
Fixes setting custom security context in container spec

Note: I have not taken any steps to test my changes outside of my unit test. If this is required please provide some guidance on the best way to do so

Which issue(s) this PR fixes:

Fixes #4881

Release Notes: Yes

@Dean-Coakley Dean-Coakley requested a review from a team as a code owner December 16, 2024 14:39
@Dean-Coakley Dean-Coakley force-pushed the fix-security-context-shudown-manager branch from 5b13980 to 34a52bf Compare December 16, 2024 14:45
@zhaohuabing
Copy link
Member

@Dean-Coakley
Please fix lint:

internal/infrastructure/kubernetes/proxy/resource_test.go:11: File is not `gci`-ed with --skip-generated -s standard -s default -s prefix(github.com/envoyproxy/gateway) (gci)
	egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
	"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/resource"

internal/infrastructure/kubernetes/proxy/resource_test.go:16: File is not `gci`-ed with --skip-generated -s standard -s default -s 

And run make gen-check and submit the changes to fix then gen check.

@Dean-Coakley
Copy link
Contributor Author

Dean-Coakley commented Dec 18, 2024

@zhaohuabing Are there some prerequisites, or do you have suggestions on how to resolve?:

$ golangci-lint version
golangci-lint has version 1.59.1 built with go1.22.3 from 1a55854a on 2024-06-09T18:08:33Z
$ go version
go version go1.23.3 linux/amd64
$ make gen-check
make[1]: Entering directory '/home/dean/go/src/github.com/dean-coakley/envoy-gateway'
===========> Running go.mod.tidy ... 
go mod tidy -compat=1.23
go: github.com/envoyproxy/gateway/internal/troubleshoot imports
	github.com/replicatedhq/troubleshoot/pkg/collect imports
	github.com/replicatedhq/troubleshoot/pkg/debug imports
	github.com/spf13/viper imports
	github.com/spf13/viper/internal/encoding/toml imports
	github.com/pelletier/go-toml/v2: mkdir /home/dean/go/pkg/mod/github.com/pelletier/go-toml: permission denied
make[1]: *** [tools/make/golang.mk:81: go.mod.tidy] Error 1
make[1]: Leaving directory '/home/dean/go/src/github.com/dean-coakley/envoy-gateway'
make: *** [Makefile:18: _run] Error 2
$ sudo make gen-check    
make[1]: Entering directory '/home/dean/go/src/github.com/dean-coakley/envoy-gateway'
/bin/bash: line 1: go: command not found
/bin/bash: line 1: go: command not found
/bin/bash: line 1: go: command not found
===========> Running go.mod.tidy ... 
go mod tidy -compat=1.23
/bin/bash: line 1: go: command not found
make[1]: *** [tools/make/golang.mk:81: go.mod.tidy] Error 127
make[1]: Leaving directory '/home/dean/go/src/github.com/dean-coakley/envoy-gateway'
make: *** [Makefile:18: _run] Error 2

@zhaohuabing
Copy link
Member

zhaohuabing commented Dec 19, 2024

github.com/pelletier/go-toml/v2: mkdir /home/dean/go/pkg/mod/github.com/pelletier/go-toml: permission denied

@Dean-Coakley

Can you try
sudo chown ubuntu -R /home/dean/go/pkg/mod
and then
make gen-check

@Dean-Coakley
Copy link
Contributor Author

Dean-Coakley commented Dec 19, 2024

My $USER is dean so I ran sudo chown dean -R /home/dean/go/pkg/mod instead which seems to resolve that issue. Thanks!

gen-check runs for a while but then I hit:

$ make gen-check
...
===========> Running go.testdata.complete ... 
go test -timeout 30s github.com/envoyproxy/gateway/internal/xds/translator --override-testdata=true
ok  	github.com/envoyproxy/gateway/internal/xds/translator	0.536s
go test -timeout 30s github.com/envoyproxy/gateway/internal/cmd/egctl --override-testdata=true
# github.com/containers/storage/drivers/btrfs
../../../../pkg/mod/github.com/containers/[email protected]/drivers/btrfs/btrfs.go:12:10: fatal error: btrfs/ioctl.h: No such file or directory
   12 | #include <btrfs/ioctl.h>
      |          ^~~~~~~~~~~~~~~
compilation terminated.
FAIL	github.com/envoyproxy/gateway/internal/cmd/egctl [build failed]
FAIL
make[1]: *** [tools/make/golang.mk:51: go.testdata.complete] Error 1
make[1]: Leaving directory '/home/dean/go/src/github.com/dean-coakley/envoy-gateway'
make: *** [Makefile:18: _run] Error 2

@zhaohuabing
Copy link
Member

../../../../pkg/mod/github.com/containers/[email protected]/drivers/btrfs/btrfs.go:12:10: fatal error: btrfs/ioctl.h: No such file or directory
12 | #include <btrfs/ioctl.h>
| ^~~~~~~~~~~~~~~

trfs lib is missing

try sudo apt-get install libbtrfs-dev

Signed-off-by: Dean Coakley <[email protected]>
@Dean-Coakley Dean-Coakley force-pushed the fix-security-context-shudown-manager branch from 269ec5d to 10f2e31 Compare December 20, 2024 17:27
Copy link

codecov bot commented Dec 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.31%. Comparing base (2a10d47) to head (45b736c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4938      +/-   ##
==========================================
- Coverage   66.34%   66.31%   -0.04%     
==========================================
  Files         209      209              
  Lines       32035    32038       +3     
==========================================
- Hits        21254    21246       -8     
- Misses       9524     9533       +9     
- Partials     1257     1259       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@zhaohuabing zhaohuabing left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants