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

extproc: load default config if file does not exist #376

Merged
merged 4 commits into from
Feb 19, 2025

Conversation

nacx
Copy link
Contributor

@nacx nacx commented Feb 19, 2025

Commit Message

extproc: load default config if file does not exist

If the configuration file does not exist, let extproc run with the default configuration. This prevents the extproc process from terminating if the config does not exist, which is a valid use case when the extproc filter is used in conjunction with other control planes.

Related Issues/PRs (if applicable)

N/A

Special notes for reviewers (if applicable)

N/A

@nacx nacx requested a review from a team as a code owner February 19, 2025 17:52
return fmt.Errorf("failed to read the config file: %w", err)
}

cw.current = string(raw)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the unmarshal method to return the raw contents as well so we don't have to read the file again when logging the diff.

type syncBuffer struct {
mu sync.RWMutex
b *bytes.Buffer
}
Copy link
Contributor Author

@nacx nacx Feb 19, 2025

Choose a reason for hiding this comment

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

I'm not sure why this didn't fail before, but when running the unit tests I got data races in the buffer, between the message logs and the call to buf.String(). This should fix that.

nacx added 3 commits February 19, 2025 13:50
Signed-off-by: Ignasi Barrera <ignasi@tetrate.io>
Signed-off-by: Ignasi Barrera <ignasi@tetrate.io>
Signed-off-by: Ignasi Barrera <ignasi@tetrate.io>
if err != nil && os.IsNotExist(err) {
// If the file does not exist, do not fail (which could lead to the extproc process to terminate)
// Instead, load the default configuration and keep running unconfigured
cfg, raw = filterapi.MustLoadDefaultConfig()
Copy link
Member

Choose a reason for hiding this comment

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

need to clear err = nil now? not exist will end up captured in if err ! = nil below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes. when this method returned an error it made sense; not anymore. fixed

Signed-off-by: Ignasi Barrera <ignasi@tetrate.io>
Copy link
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

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

Fantastic!

@mathetake mathetake merged commit 57abbd2 into envoyproxy:main Feb 19, 2025
17 checks passed
@nacx nacx deleted the extproc-cfg branch February 19, 2025 20:59
mathetake pushed a commit that referenced this pull request Feb 20, 2025
**Commit Message**

extproc: remove noise when reloading config

Reduce the noise when configuration is reloaded. Only print logs when
the configuration actually changes and do not reload the default config
on every tick.

**Related Issues/PRs (if applicable)**

Related to #376

**Special notes for reviewers (if applicable)**

N/A

---------

Signed-off-by: Ignasi Barrera <ignasi@tetrate.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants