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

Merged

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
Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com>
@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/storage@v1.55.0/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/storage@v1.55.0/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 <dean.s.coakley@gmail.com>
Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com>
@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).
Report is 18 commits behind head on main.

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!

Copy link
Contributor

@arkodg arkodg 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 !

@zhaohuabing zhaohuabing merged commit 43621b4 into envoyproxy:main Jan 6, 2025
20 of 22 checks passed
@Dean-Coakley Dean-Coakley deleted the fix-security-context-shudown-manager branch January 6, 2025 11:53
zhaohuabing pushed a commit to zhaohuabing/gateway that referenced this pull request Jan 13, 2025
…ec (envoyproxy#4938)

* Fix shutdown-manager not respecting security context of container spec

Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com>

* Update securityContext testdata

Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com>

* Lint with gci

Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com>

---------

Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com>
(cherry picked from commit 43621b4)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
zhaohuabing added a commit that referenced this pull request Jan 14, 2025
* fix: nil pointer error (#5000)

* fix: nil pointer error

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
(cherry picked from commit 10a31f1)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix: store one copy of HTTPRoute Extension Filters (#5002)

* fix: store one copy of HTTPRoute Extension Filters

Signed-off-by: Guy Daich <guy.daich@sap.com>

* fix code review comments

Signed-off-by: Guy Daich <guy.daich@sap.com>

* check if httproutefilter crd exists

Signed-off-by: Guy Daich <guy.daich@sap.com>

---------

Signed-off-by: Guy Daich <guy.daich@sap.com>
(cherry picked from commit 2a5ecaf)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix: enable ipv4 compat mode for dual stack cluster support (#5018)

enable ipv4 compat mode for dual stack cluster support

Signed-off-by: Will Tekulve <tekulve.will@gmail.com>
(cherry picked from commit e028254)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix: explicitly set ip family and family policy in gateway spec (#5019)

* explicitly set ip family and family policy

Signed-off-by: Will Tekulve <tekulve.will@gmail.com>

* add TestService cases

Signed-off-by: Will Tekulve <tekulve.will@gmail.com>
(cherry picked from commit 4d5d3f0)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix: check before setting cookie TTL in sessionPersistence (#5026)

* Check before setting Cookie TTL in Session Persistence

Fixes a null ptr exception when the cookie ttl is nil but was being
accessed without checking if its valid or not

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* simplify logic

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
(cherry picked from commit dff0531)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix: dont shift listener ports for Standalone mode (#5027)

* fix: dont shift listener ports for Standalone mode

Fixes: #4981

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* test

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* fix lint

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

---------

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
(cherry picked from commit 84f2ad2)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

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

* Fix shutdown-manager not respecting security context of container spec

Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com>

* Update securityContext testdata

Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com>

* Lint with gci

Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com>

---------

Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com>
(cherry picked from commit 43621b4)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix: use tls config from BTP when connecting to the OIDC provider's well-known endpoint. (#4857)

* add e2e test for OIDC provider with TLS

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* delete file

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix lint

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* use TLS config from BTLPolicy to fetch auth endpoint

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* refactor

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* update release note

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* update release note

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix test

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix test

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix test

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix test

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix test

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix lint

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
(cherry picked from commit 3a39c35)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Guy Daich <guy.daich@sap.com>
Signed-off-by: Will Tekulve <tekulve.will@gmail.com>
Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com>
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Co-authored-by: Guy Daich <guy.daich@sap.com>
Co-authored-by: Will Tekulve <tekulvw@users.noreply.github.com>
Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Co-authored-by: Dean Coakley <dean.s.coakley@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants