-
Notifications
You must be signed in to change notification settings - Fork 682
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
Fixed API calls for inspec compliance #537
Conversation
Hi. I am an automated pull request bot named Curry. There are commits in this pull request whose authors are not yet authorized to contribute to Chef Software, Inc. projects or are using a non-GitHub verified email address. To become authorized to contribute, you will need to sign the Contributor License Agreement (CLA) as an individual or on behalf of your company. You can read more on Chef's blog. Non-GitHub Verified CommittersThere are 1 commit author(s) whose commits are authored by a non-GitHub verified email address. Chef will have to manually verify that they are authorized to contribute. |
@@ -12,8 +12,8 @@ class API # rubocop:disable Metrics/ClassLength | |||
# logs into the server, retrieves a token and stores it locally | |||
def self.login(server, username, password, insecure) | |||
config = Compliance::Configuration.new | |||
config['server'] = server | |||
url = "#{server}/oauth/token" | |||
config['server'] = "#{server}/api" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose we add a separate path
config, to cover cases where users proxy compliance behind another other e.g. http://myserver.com/compliance
Thanks @JTabel for helping us to improve the usability. I like the idea to make |
Sure that sounds good as well. My issue was, it wasn't documented, but making configurable sounds really good. |
I updated it, so you can add --apipath as an option, which defaults to api. |
@@ -15,8 +15,10 @@ class ComplianceCLI < Inspec::BaseCLI # rubocop:disable Metrics/ClassLength | |||
desc: 'Chef Compliance Password' | |||
option :insecure, aliases: :k, type: :boolean, | |||
desc: 'Explicitly allows InSpec to perform "insecure" SSL connections and transfers' | |||
option :apipath, type: :string, default: 'api', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to default to /api
Thanks @JTabel I am happy to merge, once you signed the CLA and we use |
Sure, changed the apipath to get rid of the /. Not sure why the ICLA issue occurs, I have signed it some time ago. (https://supermarket.chef.io/users/thefurya) Maybe because i made some changes to my github account not so long ago. |
@JTabel thanks for the update, it looks great. For legal reasons, it is required that the github user that commits need to sign the ICLA. Once we have solved this, I am happy to merge your PR. |
@JTabel anything I can help to get this PR in? Would love to merge it, I am just waiting for the ICLA. |
I just checked the commit, I was missing a new email address in my profile. Not sure what to do from here on, since supermarket and github are connected and i have signed the ICLA a while ago. |
Hi. Your friendly Curry bot here. Just letting you know that all commit authors have become authorized to contribute. I have added the "Signed CLA" label to this issue so it can easily be found in the future. |
Yeah, got it fixed, my bad. But now you should be able to merge. |
Thanks @JTabel. Could you be so kind to rebase the PR to the latest master |
Awesome @JTabel |
Since the API of chef compliance is reachable via https://$YOUR_SERVER/api/$API_CALL the /api was missing in chef-compliance api calls. Using the cli for compliance, you could just give the /api part with your server url, but that is not really intuitive.