Skip to content

making the cli use AWXKIT_CREDENTIAL_FILE#9491

Merged
shanemcd merged 1 commit into
ansible:develfrom
sezanzeb:awxkit-credential-file
Oct 13, 2021
Merged

making the cli use AWXKIT_CREDENTIAL_FILE#9491
shanemcd merged 1 commit into
ansible:develfrom
sezanzeb:awxkit-credential-file

Conversation

@sezanzeb

@sezanzeb sezanzeb commented Mar 5, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor
SUMMARY
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • CLI
AWX VERSION
awx: 17.0.1
ADDITIONAL INFORMATION

setup:

> nano /awx_credentials.json
> export AWXKIT_CREDENTIAL_FILE=/awx_credentials.json
> cat /awx_credentials.json 
{
  "default": {
    "username": "testuser",
    "password": "testpassword"
  }
}

before:

> awx login
...
Error retrieving an OAuth2.0 token (<class 'awxkit.exceptions.Unauthorized'>).

after:

> awx login
{
     "token": "***"
}

@sezanzeb sezanzeb marked this pull request as draft March 5, 2021 11:24
@sezanzeb sezanzeb changed the title Awxkit credential file making the cli use AWXKIT_CREDENTIAL_FILE Mar 5, 2021
@awxbot awxbot added the type:bug label Mar 5, 2021
@softwarefactory-project-zuul

softwarefactory-project-zuul Bot commented Mar 5, 2021

Copy link
Copy Markdown
Contributor

Build failed.

@softwarefactory-project-zuul

softwarefactory-project-zuul Bot commented Mar 5, 2021

Copy link
Copy Markdown
Contributor

Build succeeded.

@sezanzeb sezanzeb marked this pull request as ready for review March 5, 2021 12:20
@jakemcdermott

jakemcdermott commented Mar 5, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

@unlikelyzero @tiagodread This addresses a bug with the awxkit config precedence. (CLI args would now override config file vars).

That's probably (?) the expected behavior but it's possible that some test suites and other tooling might implicitly rely on the unexpected (current) behavior. Can you give this a run to make sure it all still works?

@sezanzeb

sezanzeb commented Mar 5, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

tooling might implicitly rely on the unexpected (current) behavior

at least for awx login the default cli values "admin" and "password" overwrote anything stored in there, so no tooling would have been able to use the configuration file at the moment via that command as far as I can tell.

@sezanzeb

sezanzeb commented Apr 7, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Any updates?

@jakemcdermott

as already mentioned, AWXKIT_CREDENTIAL_FILE did not have any effect prior to this PR, it was completely ignored. The precedence is not the issue here.

@shanemcd

shanemcd commented Jul 22, 2021

Copy link
Copy Markdown
Member

Hello. If you could please rebase this and resolve the conflicts, we'll get this merged.

@sezanzeb

sezanzeb commented Jul 22, 2021

Copy link
Copy Markdown
Contributor Author

Thanks for getting back to this. Should be good to go now, the changes still seem to work

@sezanzeb

sezanzeb commented Aug 16, 2021

Copy link
Copy Markdown
Contributor Author

@shanemcd

@sezanzeb

sezanzeb commented Sep 9, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

tower-cli used to support config files. This merge request has unittests so that it doesn't break again, yet it has been open for 4 months now. I even merged the conflicts in on the same day of your request. To be honest, this is somewhat discouraging me to help the project out in the future... I'm not trying to be rude, but maintaining our AWX setup has caused us/me a lot of work, which adds to the frustration a bit. sorry

@sezanzeb

sezanzeb commented Sep 9, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Take care to make sure no merge commits are in the submission, and use git rebase vs. git merge for this reason. (readme)

Whoops, sorry for that. I'll just squash all commits later, should be a fresh single-commit merge request afterwards

@sezanzeb
add credential file support
cbe612b 
Signed-off-by: sezanzeb <proxima@sezanzeb.de>
@sezanzeb sezanzeb force-pushed the awxkit-credential-file branch from 0cba6eb to cbe612b Compare September 12, 2021 15:59
@sezanzeb

sezanzeb commented Sep 12, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

here you are, the PR is now a single commit (also with --signoff)

AlanCoding
AlanCoding reviewed Oct 2, 2021
View reviewed changes
Comment thread awxkit/awxkit/cli/format.py
auth.add_argument(
'--conf.password',
default=env.get('CONTROLLER_PASSWORD', env.get('TOWER_PASSWORD', 'password')),
default=env.get('CONTROLLER_PASSWORD', env.get('TOWER_PASSWORD', config_password)),

@AlanCoding AlanCoding Oct 2, 2021

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.

the default won't show in the help text, will it?

(sorry for the surface-level question, I'm just starting to go over this)

@sezanzeb sezanzeb Oct 2, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no, not as long argparse.ArgumentDefaultsHelpFormatter is not used

image

shanemcd
shanemcd approved these changes Oct 13, 2021
View reviewed changes

@shanemcd shanemcd 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.

Thanks for the contribution!

@shanemcd shanemcd merged commit 517f1d7 into ansible:devel Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlanCoding AlanCoding AlanCoding left review comments

@shanemcd shanemcd shanemcd approved these changes

@beeankha beeankha Awaiting requested review from beeankha

Assignees

No one assigned

Labels

component:cli type:bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sezanzeb @jakemcdermott @shanemcd @AlanCoding @awxbot