-
Notifications
You must be signed in to change notification settings - Fork 10
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
Make our container image "distroless" #15
Changes from all commits
adea7b5
04e0cb5
2958bac
7f1c86c
eb08a05
e0b9ce7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
```release-note:enhancement | ||
Container image is now "distroless" for better security | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,11 +32,12 @@ var ( | |
logLevel string | ||
logJSON bool | ||
|
||
nodeName string | ||
nodeID string | ||
serviceID string | ||
namespace string | ||
partition string | ||
nodeName string | ||
nodeID string | ||
serviceID string | ||
serviceIDPath string | ||
namespace string | ||
partition string | ||
|
||
credentialType string | ||
token string | ||
|
@@ -93,6 +94,7 @@ func init() { | |
flag.StringVar(&nodeName, "service-node-name", "", "The name of the Consul node to which the proxy service instance is registered.") | ||
flag.StringVar(&nodeID, "service-node-id", "", "The ID of the Consul node to which the proxy service instance is registered.") | ||
flag.StringVar(&serviceID, "proxy-service-id", "", "The proxy service instance's ID.") | ||
flag.StringVar(&serviceIDPath, "proxy-service-id-path", "", "The path to a file containing the proxy service instance's ID.") | ||
flag.StringVar(&namespace, "service-namespace", "", "The Consul Enterprise namespace in which the proxy service instance is registered.") | ||
flag.StringVar(&partition, "service-partition", "", "The Consul Enterprise partition in which the proxy service instance is registered.") | ||
|
||
|
@@ -155,6 +157,7 @@ func main() { | |
return | ||
} | ||
|
||
readServiceIDFromFile() | ||
validateFlags() | ||
|
||
consuldpCfg := &consuldp.Config{ | ||
|
@@ -249,3 +252,19 @@ func main() { | |
log.Fatal(err) | ||
} | ||
} | ||
|
||
// readServiceIDFromFile reads the service ID from the file specified by the | ||
// -proxy-service-id-path flag. | ||
// | ||
// We do this here, rather than in the consuldp package's config handling, | ||
// because this option only really makes sense as a CLI flag (and we handle | ||
// all flag parsing here). | ||
Comment on lines
+260
to
+261
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it'd make sense to add this to the config file, what do you think? |
||
func readServiceIDFromFile() { | ||
if serviceID == "" && serviceIDPath != "" { | ||
id, err := os.ReadFile(serviceIDPath) | ||
if err != nil { | ||
log.Fatalf("failed to read given -proxy-service-id-path: %v", err) | ||
} | ||
serviceID = string(id) | ||
} | ||
} | ||
Comment on lines
+256
to
+270
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @ishustava @thisisnotashwin 👋🏻 Do you have an explanation of why consul-k8s needs this, rather than putting the service ID directly in the CLI flags, that I could put here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In K8s, we often have a different container responsible for getting the service ID to begin with. Information across containers can only be shared via files on a shared volume. It makes it significantly easier to ready what is going on if we are dealing with filepath. Putting the service ID directly to the CLI would require reading the contents of that file anyway. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to use the (smaller)
static-debian11
image, but unfortunately, Envoy dynamically links system libraries so this is not possible.