Skip to content

Allow customizing sanitization rules for /env #11264

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

Closed
wants to merge 1 commit into from
Closed

Allow customizing sanitization rules for /env #11264

wants to merge 1 commit into from

Conversation

pbetkier
Copy link

@pbetkier pbetkier commented Dec 5, 2017

This PR introduces more flexible sanitization rules for /env actuator endpoint.

Sanitization rules can be customized by registering one or more beans of type EnvironmentEndpoint.SanitizeFilter. Such filters decide whether to sanitize a given value basing on its property name and the property source it originates from. By default there is only one filter, KeyBlacklistSanitizeFilter, which contains the already existing logic of sanitizing based on the matching key names.

I made a couple of backwards-incompatible changes which I assumed are OK at this release stage. I pointed them in the PR comments. I also haven't wrote the docs for this feature assuming it won't be commonly used.

@philwebb @spencergibb you may be interested in this :)

Fixes gh-6587

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 5, 2017
@@ -195,8 +206,8 @@ private void extract(String root, Map<String, PropertySource<?>> map,
}
}

public Object sanitize(String name, Object object) {
return this.sanitizer.sanitize(name, object);
private Object sanitize(PropertyNameAndSource source, Object object) {
Copy link
Author

Choose a reason for hiding this comment

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

Couldn't find out why this method was public, made it private since I changed the signature anyway. If we make this public again then PropertyNameAndSource must have a public constructor which it now doesn't.

}
}
}
return null;
}

protected String convertResolvedValueToString(String placeholder, Object value, PropertySource<?> source) {
Copy link
Author

Choose a reason for hiding this comment

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

I changed the method that we allow to override, making the previous one private. I thought it's not a problem since resolvePlaceholder was made protected only recently (commit eb045f1).

}

public void setKeysToSanitize(String... keysToSanitize) {
Copy link
Author

Choose a reason for hiding this comment

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

Again, dropped the setter in favour of constructor.

return endpoint;
KeyBlacklistSanitizeFilter keyMatchFilter = (keysToSanitize != null)
? KeyBlacklistSanitizeFilter.matchingKeys(keysToSanitize)
: KeyBlacklistSanitizeFilter.matchingDefaultKeys();
Copy link
Author

@pbetkier pbetkier Dec 5, 2017

Choose a reason for hiding this comment

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

I wanted to create the instance of KeyBlacklistSanitizeFilter as a bean that we would autowire here, but couldn't express the condition "register the filter bean only if environment endpoint is enabled". Is there a way for expressing that given that the filter bean must be registered before EnvironmentEndpoint?

@pbetkier
Copy link
Author

@philwebb can you have a look at this?

@pbetkier
Copy link
Author

pbetkier commented Mar 9, 2018

@philwebb @spencergibb it's been 3 months, are you still interested in this feature? I won't bother resolving conflicts if you're not.

@philwebb
Copy link
Member

Hi @pbetkier,

Thanks for the PR and sorry I didn't get back to you earlier. We're planning to start the planning process for Spring Boot 2.1 soon and I'd like to review this at that point. There's no need to resolve conflicts now, we can deal with that when we merge.

FWIW I really like the idea of supporting more flexible sanitization rules, we just ran out of time to get this into 2.0.

@mbhave
Copy link
Contributor

mbhave commented Aug 31, 2021

@pbetkier Sorry for the delay in getting to this. Given that the code has moved on quite a bit since the original PR, we decided to approach this in a different manner. This commit adds a new method to the Sanitizer and leaves the existing API as is. Users can plug in a custom SanitizingFunction which can make use of the propertySource or key. Thanks again and sorry for the wasted time.

@mbhave mbhave closed this Aug 31, 2021
@mbhave mbhave added status: declined A suggestion or change that we don't feel we should currently apply and removed type: enhancement A general enhancement for: merge-with-amendments Needs some changes when we merge labels Aug 31, 2021
@mbhave mbhave removed their assignment Aug 31, 2021
@snicoll snicoll removed this from the General Backlog milestone Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to sanitize all keys in /env from a particular property source
5 participants