Skip to content

Enhancements: Example Config File, Default Path, and Error Handling Improvements #199

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

Merged
merged 9 commits into from
Feb 5, 2024

Conversation

maxlambrecht
Copy link
Member

This PR add improvements to the java-spiffe-helper:

  • Example Config File: Added an example config file for user convenience.
  • Default Path for --config Parameter.
  • Improved Error Handling: Enhanced error messages and added exception stack trace logging for better error diagnosis.
  • Minor refactors and improvements in the code.

@kfox1111
Copy link

I can't tell from a quick skim of the patch, where its going to try to read the default config file from in a containerized environment.

Where is it going to look?

@maxlambrecht
Copy link
Member Author

I can't tell from a quick skim of the patch, where its going to try to read the default config file from in a containerized environment.

Where is it going to look?

In a containerized environment, the code will attempt to read the default config file from conf/java-spiffe-helper.conf, and it will look for this file relative to the current working directory.

@maxlambrecht maxlambrecht force-pushed the add-default-helper-config branch from 19e9113 to 1c22ad6 Compare January 25, 2024 23:22
@kfox1111
Copy link

I can't tell from a quick skim of the patch, where its going to try to read the default config file from in a containerized environment.
Where is it going to look?

In a containerized environment, the code will attempt to read the default config file from conf/java-spiffe-helper.conf, and it will look for this file relative to the current working directory.

It would be better I think if the user didn't have to read a Dockerfile to try and find where that is? Defaulting to something more well defined like /etc/java-spiffe-helper.conf would help make it more concrete out of the box?

@maxlambrecht
Copy link
Member Author

I can't tell from a quick skim of the patch, where its going to try to read the default config file from in a containerized environment.
Where is it going to look?

In a containerized environment, the code will attempt to read the default config file from conf/java-spiffe-helper.conf, and it will look for this file relative to the current working directory.

It would be better I think if the user didn't have to read a Dockerfile to try and find where that is? Defaulting to something more well defined like /etc/java-spiffe-helper.conf would help make it more concrete out of the box?

The java-spiffe-helper can be used both in containers and as a standalone JAR. For ease of use, it defaults to the conf/java-spiffe-helper.conf configuration file, as outlined in the README in this PR. This is makes it straightforward to use right away:

> ./gradlew build
> java -jar java-spiffe-helper/build/libs/java-spiffe-helper-0.8.4-osx-aarch_64.jar

@kfox1111
Copy link

If you want to do that for non containers, then I think the command suggestion in #187 is still the right thing to do for the containerized side then.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this file have extension .properties rather than .conf? Java properties files typically have the .properties suffix.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

Signed-off-by: Max Lambrecht <maxlambrecht@gmail.com>
Signed-off-by: Max Lambrecht <maxlambrecht@gmail.com>
Signed-off-by: Max Lambrecht <maxlambrecht@gmail.com>
Signed-off-by: Max Lambrecht <maxlambrecht@gmail.com>
Signed-off-by: Max Lambrecht <maxlambrecht@gmail.com>
Signed-off-by: Max Lambrecht <maxlambrecht@gmail.com>
Signed-off-by: Max Lambrecht <maxlambrecht@gmail.com>
Signed-off-by: Max Lambrecht <maxlambrecht@gmail.com>
@maxlambrecht maxlambrecht force-pushed the add-default-helper-config branch from 1c22ad6 to e366d0d Compare January 31, 2024 13:10
@maxlambrecht maxlambrecht merged commit 666766a into spiffe:main Feb 5, 2024
@maxlambrecht maxlambrecht deleted the add-default-helper-config branch February 5, 2024 15:13
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.

None yet

3 participants