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.

Sign up for GitHub

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

Jump to bottom

[DO NOT MERGE] Rename InSpec Attributes to Inputs #401

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[DO NOT MERGE] Rename InSpec Attributes to Inputs #401

wants to merge 5 commits into from

Conversation

clintoncwolfe
Copy link
Contributor

@clintoncwolfe clintoncwolfe commented Dec 3, 2019 •
edited
Loading

Description

As one of the last remaining parts of the renaming of InSpec Attributes to Inputs, this PR renames the default['audit']['attributes'] chef infra attribute to default['audit']['inputs'].

default['audit']['attributes'] is retained, but any values set there are merged into default['audit']['inputs'] and a warning is emitted.

Additionally, the attributes
node['audit']['attributes_save'] has been renamed to node['audit']['inputs_save'] and
node['audit']['chef_node_attribute_enabled'] to node['audit']['chef_node_input_enabled']. At this time no backwards compatibility is in place for these two features, but that could be added if desired.

Should this be on a major release?

Issues Resolved

Fixes #360

Also gets the attributes test suite passing, which had been failing.

Check List

@clintoncwolfe clintoncwolfe requested a review from a team December 3, 2019 19:25
@clintoncwolfe
Update README with input rename
53c79fb 
Signed-off-by: Clinton Wolfe <[email protected]>
@clintoncwolfe clintoncwolfe added Aspect: Integration Works correctly with other projects or systems. Type: Breaking Change Changes the way existing features work in a way that affects users. Aspect: UI How users interact with the interface of the project, with a focus on interaction and visual design. Component: Cookbook Attributes labels Dec 3, 2019
@vjeffrey
Copy link

vjeffrey commented Dec 3, 2019

we'll need to do some work in automate to ensure we're looking for that inputs field: chef/automate#194

@davymcaleer
Copy link

Let's find out the extent of any possible breakage risk to existing customers before this one progresses - should hold for now

@clintoncwolfe
Copy link
Contributor Author

This PR exists to look for feedback and alert stakeholders to possible disruption. It's not a particularly urgent change. We'll hold until the change and its impact is better understood, and until there are any downstream changes needed in other products have been implemented.

That said, technical reviewers, please do give feedback: is this sufficiently backwards compatible? will this impact Automate on node save? Can that be avoided? Do we want a major version bump for this change?

@clintoncwolfe clintoncwolfe changed the title Rename InSpec Attributes to Inputs [DO NOT MERGE] Rename InSpec Attributes to Inputs Dec 5, 2019
@clintoncwolfe
Copy link
Contributor Author

DO NOT MERGE flagged per discussion

@james-stocks
Copy link

At least a couple of concerns:

  • Supporting profiles that we ship that still refer to attributes
  • Supporting profiles that users own that still refer to attributes
  • Supporting users that upgrade their audit cookbook version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexpop alexpop Awaiting requested review from alexpop

@btm btm Awaiting requested review from btm

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Aspect: Integration Works correctly with other projects or systems. Aspect: UI How users interact with the interface of the project, with a focus on interaction and visual design. Component: Cookbook Attributes Type: Breaking Change Changes the way existing features work in a way that affects users.
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Change attributes attribute to inputs
4 participants
@clintoncwolfe @vjeffrey @davymcaleer @james-stocks