Skip to content
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

Adds Chef/Inspec User-Agent to Azure API calls #120

Merged
28 commits merged into from
Sep 18, 2018
Merged

Adds Chef/Inspec User-Agent to Azure API calls #120

28 commits merged into from
Sep 18, 2018

Conversation

ghost
Copy link

@ghost ghost commented Aug 20, 2018

Description

Adds Chef Open Source UUID as a header on all API calls to Azure, to measure usage/impact.

Check List

David McCown and others added 23 commits July 10, 2018 16:26
This change replaces our connection logic with the InSpec backend
(backed by Train) to handle connections. By doing so, we can now support
using a credentials file, MSI connector as well as shell vars.

I'm also adding another terraform target to start up a linux virtual
machine with MSI enabled. You will need to add that MSI account with the
contributor role to your subscription for MSI to work against the REST
API.

To start up the MSI vm:

`rake msi_vm tf:apply`

You will need to add a public ssh key to your `.envrc` file as well. An
example is included in `.envrc-example`.

Note: If you run integration tests while you have the MSI vm running
tests will fail. The tests were written with the assumption that there
would be only two vms.

Signed-off-by: David McCown <dmccown@chef.io>
This change replaces our connection logic with the InSpec backend
(backed by Train) to handle connections. By doing so, we can now support
using a credentials file, MSI connector as well as shell vars.

I'm also adding another terraform target to start up a linux virtual
machine with MSI enabled. You will need to add that MSI account with the
contributor role to your subscription for MSI to work against the REST
API.

To start up the MSI vm:

`rake msi_vm tf:apply`

You will need to add a public ssh key to your `.envrc` file as well. An
example is included in `.envrc-example`.

Note: If you run integration tests while you have the MSI vm running
tests will fail. The tests were written with the assumption that there
would be only two vms.

Signed-off-by: David McCown <dmccown@chef.io>
Signed-off-by: David McCown <dmccown@chef.io>
This change replaces our connection logic with the InSpec backend
(backed by Train) to handle connections. By doing so, we can now support
using a credentials file, MSI connector as well as shell vars.

I'm also adding another terraform target to start up a linux virtual
machine with MSI enabled. You will need to add that MSI account with the
contributor role to your subscription for MSI to work against the REST
API.

To start up the MSI vm:

`rake msi_vm tf:apply`

You will need to add a public ssh key to your `.envrc` file as well. An
example is included in `.envrc-example`.

Note: If you run integration tests while you have the MSI vm running
tests will fail. The tests were written with the assumption that there
would be only two vms.

Signed-off-by: David McCown <dmccown@chef.io>
This change replaces our connection logic with the InSpec backend
(backed by Train) to handle connections. By doing so, we can now support
using a credentials file, MSI connector as well as shell vars.

I'm also adding another terraform target to start up a linux virtual
machine with MSI enabled. You will need to add that MSI account with the
contributor role to your subscription for MSI to work against the REST
API.

To start up the MSI vm:

`rake msi_vm tf:apply`

You will need to add a public ssh key to your `.envrc` file as well. An
example is included in `.envrc-example`.

Note: If you run integration tests while you have the MSI vm running
tests will fail. The tests were written with the assumption that there
would be only two vms.

Signed-off-by: David McCown <dmccown@chef.io>
Signed-off-by: David McCown <dmccown@chef.io>
* Replaces all default `client` names with `management` to indicate what
  route the resource is interacting with

Co-authored-by: Ruairi Fennell <rfennell@chef.io>
Co-authored-by: David McCown <dmccown@chef.io>

Signed-off-by: David McCown <dmccown@chef.io>
This change replaces our connection logic with the InSpec backend
(backed by Train) to handle connections. By doing so, we can now support
using a credentials file, MSI connector as well as shell vars.

I'm also adding another terraform target to start up a linux virtual
machine with MSI enabled. You will need to add that MSI account with the
contributor role to your subscription for MSI to work against the REST
API.

To start up the MSI vm:

`rake msi_vm tf:apply`

You will need to add a public ssh key to your `.envrc` file as well. An
example is included in `.envrc-example`.

Note: If you run integration tests while you have the MSI vm running
tests will fail. The tests were written with the assumption that there
would be only two vms.

Signed-off-by: David McCown <dmccown@chef.io>
This change replaces our connection logic with the InSpec backend
(backed by Train) to handle connections. By doing so, we can now support
using a credentials file, MSI connector as well as shell vars.

I'm also adding another terraform target to start up a linux virtual
machine with MSI enabled. You will need to add that MSI account with the
contributor role to your subscription for MSI to work against the REST
API.

To start up the MSI vm:

`rake msi_vm tf:apply`

You will need to add a public ssh key to your `.envrc` file as well. An
example is included in `.envrc-example`.

Note: If you run integration tests while you have the MSI vm running
tests will fail. The tests were written with the assumption that there
would be only two vms.

Signed-off-by: David McCown <dmccown@chef.io>
Signed-off-by: David McCown <dmccown@chef.io>
* Replaces all default `client` names with `management` to indicate what
  route the resource is interacting with

Co-authored-by: Ruairi Fennell <rfennell@chef.io>
Co-authored-by: David McCown <dmccown@chef.io>

Signed-off-by: David McCown <dmccown@chef.io>
Signed-off-by: David McCown <dmccown@chef.io>
Signed-off-by: David McCown <dmccown@chef.io>
Signed-off-by: Ruairi Fennell <rfennell@chef.io>
Signed-off-by: Ruairi Fennell <rfennell@chef.io>
Signed-off-by: Ruairi Fennell <rfennell@chef.io>
Signed-off-by: Ruairi Fennell <rfennell@chef.io>
@ghost ghost requested review from dmccown, russellseymour and davymcaleer August 20, 2018 16:43
@davymcaleer
Copy link

davymcaleer commented Aug 20, 2018

@r-fennell the User Agent should be:
INSPEC_USER_AGENT = 'Chef-Inspec-18d63047-6cdf-4f34-beed-62f01fc73fc2'
No the pid was actually the ProductID, in this case InSpec

@ghost
Copy link
Author

ghost commented Aug 20, 2018

Should Chef removed completely?

@@ -5,6 +5,10 @@

module Azure
class Rest
USER_AGENT = 'User-Agent'
INSPEC_USER_AGENT = 'Chef-Inspec-pid-18d63047-6cdf-4f34-beed-62f01fc73fc2'
private_constant :USER_AGENT, :INSPEC_USER_AGENT
Copy link
Contributor

Choose a reason for hiding this comment

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

Strings are already frozen on line 1

# frozen_string_literal: true
and these vars should be private by default. I don't think you need line 10.

Choose a reason for hiding this comment

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

Sorry @r-fennell - re-added Chef above - for clarity just removing the pid string:

INSPEC_USER_AGENT = 'Chef-Inspec-18d63047-6cdf-4f34-beed-62f01fc73fc2'

Copy link
Author

Choose a reason for hiding this comment

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

@dmccown I can't find any documentation indicating default scope of constants is private. I used this and this as reference. Is it a new ruby version change?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was wrong in assuming scope, thanks for pointing that out.

Signed-off-by: Ruairi Fennell <rfennell@chef.io>
@@ -5,6 +5,9 @@

module Azure
class Rest
USER_AGENT = 'User-Agent'
INSPEC_USER_AGENT = 'Chef-Inspec-18d63047-6cdf-4f34-beed-62f01fc73fc2'

Choose a reason for hiding this comment

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

this must be:
INSPEC_USER_AGENT = 'pid-18d63047-6cdf-4f34-beed-62f01fc73fc2'
you can add others if needed

Choose a reason for hiding this comment

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

@bmoore-msft "you can add others if needed" so we should generate GUIDs for our specific subproducts rather than tagging them in the agent?

Choose a reason for hiding this comment

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

We need exactly one string in the "pid-GUID" format... if there is other info you want to derive from the UA (not related to our tracking) then, you can still add it to the UA, but it will be ignored in the query. That help?

Signed-off-by: Ruairi Fennell <rfennell@chef.io>
@ghost ghost changed the base branch from dmccown/replace-connection-with-train to master September 12, 2018 20:59
@ghost
Copy link
Author

ghost commented Sep 12, 2018

@trevorghess Sorry it took so long to get back to this. Let me know if anything else needs changed!

Copy link
Contributor

@russellseymour russellseymour left a comment

Choose a reason for hiding this comment

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

👍

Ruairi Fennell added 2 commits September 18, 2018 10:56
Signed-off-by: Ruairi Fennell <rfennell@chef.io>
@ghost ghost merged commit 25cb68f into master Sep 18, 2018
@ghost ghost deleted the Inspec-User-Agent branch September 18, 2018 10:03
This pull request was closed.
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.

5 participants