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

missing the envoy-gateaway-config for certgen command #4935

Open
qicz opened this issue Dec 16, 2024 · 2 comments
Open

missing the envoy-gateaway-config for certgen command #4935

qicz opened this issue Dec 16, 2024 · 2 comments
Assignees
Labels
kind/bug Something isn't working
Milestone

Comments

@qicz
Copy link
Member

qicz commented Dec 16, 2024

Description:

What issue is being seen? Describe what should be happening instead of
the bug, for example: Envoy should not crash, the expected value isn't
returned, etc.

Currently, the certgen job does not mount the envoy-gateway-config, so it can not read the extension config from envoy-gateway-config.

Repro steps:

Include sample requests, environment, etc. All data and inputs
required to reproduce the bug.

Note: If there are privacy concerns, sanitize the data prior to
sharing.

Environment:

Include the environment like gateway version, envoy version and so on.

Logs:

Include the access logs and the Envoy logs.

@qicz qicz added the kind/bug Something isn't working label Dec 16, 2024
@qicz qicz self-assigned this Dec 16, 2024
@qicz
Copy link
Member Author

qicz commented Dec 17, 2024

the certgen command just read the OverwriteControlPlaneCerts from envoy-gateway-config. so there are three options,

opt1: init with true for the OverwriteControlPlaneCerts

// DefaultEnvoyGatewayProvider returns a new EnvoyGatewayProvider with default configuration parameters.
func DefaultEnvoyGatewayProvider() *EnvoyGatewayProvider {
return &EnvoyGatewayProvider{
Type: ProviderTypeKubernetes,
Kubernetes: &EnvoyGatewayKubernetesProvider{
LeaderElection: DefaultLeaderElection(),
},
}
}

to

// DefaultEnvoyGatewayProvider returns a new EnvoyGatewayProvider with default configuration parameters.
func DefaultEnvoyGatewayProvider() *EnvoyGatewayProvider {
	return &EnvoyGatewayProvider{
		Type: ProviderTypeKubernetes,
		Kubernetes: &EnvoyGatewayKubernetesProvider{
			LeaderElection: DefaultLeaderElection(),
			OverwriteControlPlaneCerts: true,
		},
	}
}

opt2: support OverwriteControlPlaneCerts logic for certgen command directly

// outputCertsForKubernetes outputs the provided certs to a secret in namespace ns.
func outputCertsForKubernetes(ctx context.Context, cli client.Client, cfg *config.Server, certs *crypto.Certificates) error {
var updateSecrets bool
if cfg.EnvoyGateway != nil &&
cfg.EnvoyGateway.Provider != nil &&
cfg.EnvoyGateway.Provider.Kubernetes != nil &&
cfg.EnvoyGateway.Provider.Kubernetes.OverwriteControlPlaneCerts != nil &&
*cfg.EnvoyGateway.Provider.Kubernetes.OverwriteControlPlaneCerts {
updateSecrets = true
}
secrets, err := kubernetes.CreateOrUpdateSecrets(ctx, cli, kubernetes.CertsToSecret(cfg.Namespace, certs), updateSecrets)

the line 90, set the parameter with true directly.

 secrets, err := kubernetes.CreateOrUpdateSecrets(ctx, cli, kubernetes.CertsToSecret(cfg.Namespace, certs), true) 

opt3: mount the envoy-gateway-config like EG deployment to cergen job for reading the OverwriteControlPlaneCerts.

@arkodg @zirain any suggestion? I am +1 for opt2

@arkodg
Copy link
Contributor

arkodg commented Jan 7, 2025

thanks for flagging this @qicz
I prefer opt 3, since the code today is reusing the same functions
will need to add something similar to

for certgen job template
cc @guydc as this relates to #4891

@arkodg arkodg added this to the v1.3.0 milestone Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants