Updated readme with various newer information and cleanups #71
Conversation
The README was seriously out of date and did not contain relevant info for the current version of the code. The new changes should get us up to date and reduce problems with deployments/usages of this library.
We had a lot of inconsistent tab/whitespace usage all over the place so now we have fixed those to only use spaces.
|
Using JAR file, step 3: markdown error I ran through the README and it looks good! The flow was a lot easier. I haven't run through the Maven steps though |
README.md
Outdated
|
|
||
| `CONJUR_AUTHN_API_KEY` - user/host API key | ||
| The following environment variables need to be included in the apps runtime environment in |
There was a problem hiding this comment.
| The following environment variables need to be included in the apps runtime environment in | |
| The following environment variables may need to be included in the app's runtime environment in |
There was a problem hiding this comment.
This is intentional. They don't have to be included - they can be supplemented by system properties or inlined code
There was a problem hiding this comment.
ok, updated with a qualifier and still want to get that apostrophe in :)
README.md
Outdated
| // Authenticate using provided username/hostname and password/API key | ||
| Conjur conjur = new Conjur('host/host-id', 'api-key'); | ||
| // or | ||
| Conjur conjur = new Conjur('username', 'password'); |
There was a problem hiding this comment.
does it really work with password or API key? if it's a password, the client would have to know to login first and then authenticate with the API key, right?
maybe that is how it works, but I'm not sure from the method signature that the function would know whether it has a password or API key to make the decision about what to do, and afaik the authenticate route requires an API key
There was a problem hiding this comment.
Hmm I'll look into this - there's a confusion I think in some of our "api" vs "password" wording
There was a problem hiding this comment.
I don't have a good setup to get a host working but I think the phrasing here is:
Conjur('host/host-id', 'api-key'); will try logging in with host as user and api-key as password
Conjur conjur = new Conjur('username', 'password'); will try logging in with username as user and password as password.
There was a problem hiding this comment.
have you tried it with username / password? if it works, then I suppose this can stay as-is
There was a problem hiding this comment.
I've used the admin account (which I suppose has an "api key" even though it's realistically the password(?) ). let me make a real user and set a password to see what's going on here - I might just be confused.
This was due to issues with JDK and can be worked around by setting a few config variables on the plugin. Upstream: https://bugs.openjdk.java.net/browse/JDK-8212233
README.md
Outdated
| 3. Run `mvn package -DskipTests` to generate a JAR file. The output `.jar` files will be located | ||
| in the `target` directory of the repo | ||
|
|
||
| _NOTE:_ we ran `mvn package` without running the integration tests, since these require access |
There was a problem hiding this comment.
| _NOTE:_ we ran `mvn package` without running the integration tests, since these require access | |
| _NOTE:_ the above command runs `mvn package` without running the integration tests, since these require access |
| CONJUR_AUTHN_API_KEY=sb0ncv1yj9c4w2e9pb1a2s | ||
| CONJUR_APPLIANCE_URL=https://conjur.myorg.com/api | ||
| ```sh-session | ||
| mvn exec:java \ |
There was a problem hiding this comment.
If they aren't sensitive, it's probably move common to provide them in the pom.xml file itself within ` element. See docs.
There was a problem hiding this comment.
True but I didn't have time to investigate if those persist in the created artifacts (generated jar). That part may be for a followup issue.
There was a problem hiding this comment.
If they are put into the parent project pom.xml, they should apply to included compiled, dependencies. But, still, fine to leave for now.
There was a problem hiding this comment.
Fair enough - it's definitely something that could be expanded here (once we have a bit more time on it)
Some clarifications to the wording and formatting was done to the README as part of this commit.
|
Code Climate has analyzed commit 8ce3cb6 and detected 107 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
The README was seriously out of date and did not contain relevant info
for the current version of the code. The new changes should get us up to
date and reduce problems with deployments/usages of this library.
Also fixed issues w/ javadoc generation
Fixes #70
Fixes #50
Fixes #39
Fixes #18
Connected to #18