feat(maven): Remove Gradle properties after publishing - #276
Conversation
This may prevent exposing the file, which contains the OSSRH credentials.
| */ | ||
| public deleteUserGradlePropsFile(): Promise<void> { | ||
| return fsPromises.unlink( | ||
| join(this.getGradleHomeDir(), GRADLE_PROPERTIES_FILENAME) |
There was a problem hiding this comment.
Who creates this file? If it is the closeAndReleaseRepository script, then shouldn't this be its responsibility (to remove the file I mean)
There was a problem hiding this comment.
It's created by the target itself (by the method right above it)
There was a problem hiding this comment.
Then at this point, I'd convert this into something like withTempDir with auto-clean up even in case of exceptions for safety. This is much like a Python Context Manager
BYK
left a comment
There was a problem hiding this comment.
Pushing back for even safer code 馃榾
| */ | ||
| public deleteUserGradlePropsFile(): Promise<void> { | ||
| return fsPromises.unlink( | ||
| join(this.getGradleHomeDir(), GRADLE_PROPERTIES_FILENAME) |
There was a problem hiding this comment.
Then at this point, I'd convert this into something like withTempDir with auto-clean up even in case of exceptions for safety. This is much like a Python Context Manager
BYK
left a comment
There was a problem hiding this comment.
Postponing the context manager idea to a follow-up. Looks good as it is, thanks a lot!
|
@iker-barriocanal my suggestion was not really to remove the file, but to erase the content of the written properties, this is a global file that has also users configuration, if you delete the file, I will have to copy/paste all my user configuration again |
|
@marandaneto I think we can incorporate this feedback into the planned follow-up: use a context manager-like function which keeps a snapshot of the old file around and restores it back once the operation finishes. |
|
@marandaneto if that user configuration is relevant for this target, the target itself should be able to get it in some way. When this gets moved to GHA anything that the target isn't able to get won't be in that config file. |
|
@iker-barriocanal no, its not relevant for the target, but the file is a global configuration file for Gradle itself, so it could be from different projects etc, eg: a=1 now you add so: a=1 when you delete the file, my projects in need of @BYK suggestion of keeping a snapshot of the file is great, restoring to the previous state, I'd rather say it's a severe bug. |
Removing the
gradle.propertiesafter running the maven target may prevent exposing it, which contains the OSSRH credentials (see #272). There also are minor changes to address some feedback in #270.