Skip to content

Fix props file newline potential bugs - #317

Merged
DomGarguilo merged 2 commits into
apache:mainfrom
DomGarguilo:newlineConfFix
Sep 18, 2026
Merged

DomGarguilo merged 2 commits into
apache:mainfrom
DomGarguilo:newlineConfFix

Conversation

@DomGarguilo

Copy link
Copy Markdown
Member

While doing some testing, I added a property to accumulo.properties via an echo ... >> command, assuming there was a newline at the end of the file. There wasn't, so it appended the property to the value of the final line, which messed things up.

This PR adds a newline to the props file. I also did some digging and found that the encryption plugin relied on that newline too. Its first property was getting tacked onto the end of the last line, so instance.crypto.opts.key.uri never actually got set. I changed that part of the plugin to write its props as a block with a blank line and a comment before it, so it doesn't rely on the newline anymore (even though it is there now).

While I was in there, I also noticed the plugin had the accumulo-core jar version hardcoded to 2.x, so I made that version agnostic.

@DomGarguilo DomGarguilo self-assigned this Sep 15, 2026

@ctubbsii ctubbsii left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Your echos in a block with a consolidated file append redirect was good, but I think this heredoc version I added is a little more readable... don't have to deal with quotes as much, and no redundant echo commands.

@DomGarguilo

Copy link
Copy Markdown
Member Author

Your echos in a block with a consolidated file append redirect was good, but I think this heredoc version I added is a little more readable... don't have to deal with quotes as much, and no redundant echo commands.

Yea your version is better. Thanks for the improvements.

@DomGarguilo
DomGarguilo merged commit 6ecd801 into apache:main Sep 18, 2026
2 checks passed
@DomGarguilo
DomGarguilo deleted the newlineConfFix branch September 18, 2026 15:27
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