Remove 'attributes' attribute from cookbook metadata

We have a foodcritic rule to tell people not to do this
It's not being used anywhere and is just a pile of code we run for no reason
We should start warning that it's an unknown attribute when cookbooks contain these as it's wasted effort on the part of cookbook authors

Signed-off-by: Tim Smith <tsmith@chef.io>

4 files changed, 4 insertions, 350 deletions

diff --git a/lib/chef/cookbook/metadata.rb b/lib/chef/cookbook/metadata.rb
index 116544c7f8..5db862e500 100644
--- a/lib/chef/cookbook/metadata.rb
+++ b/lib/chef/cookbook/metadata.rb
@@ -44,7 +44,6 @@ class Chef
       PLATFORMS              = "platforms".freeze
       DEPENDENCIES           = "dependencies".freeze
       PROVIDING              = "providing".freeze
-      ATTRIBUTES             = "attributes".freeze
       RECIPES                = "recipes".freeze
       VERSION                = "version".freeze
       SOURCE_URL             = "source_url".freeze
@@ -56,9 +55,8 @@ class Chef
 
       COMPARISON_FIELDS = [ :name, :description, :long_description, :maintainer,
                             :maintainer_email, :license, :platforms, :dependencies,
-                            :providing, :attributes, :recipes, :version,
-                            :source_url, :issues_url, :privacy, :chef_versions, :ohai_versions,
-                            :gems ].freeze
+                            :providing, :recipes, :version, :source_url, :issues_url,
+                            :privacy, :chef_versions, :ohai_versions, :gems ].freeze
 
       VERSION_CONSTRAINTS = { depends: DEPENDENCIES,
                               provides: PROVIDING,
@@ -71,7 +69,6 @@ class Chef
       attr_reader :platforms
       attr_reader :dependencies
       attr_reader :providing
-      attr_reader :attributes
       attr_reader :recipes
 
       # @return [Array<Gem::Dependency>] Array of supported Chef versions
@@ -104,7 +101,6 @@ class Chef
         @platforms = Mash.new
         @dependencies = Mash.new
         @providing = Mash.new
-        @attributes = Mash.new
         @recipes = Mash.new
         @version = Version.new("0.0.0")
         @source_url = ""
@@ -385,51 +381,6 @@ class Chef
         end
       end
 
-      # Adds an attribute that a user needs to configure for this cookbook. Takes
-      # a name (with the / notation for a nested attribute), followed by any of
-      # these options
-      #
-      #   display_name<String>:: What a UI should show for this attribute
-      #   description<String>:: A hint as to what this attr is for
-      #   choice<Array>:: An array of choices to present to the user.
-      #   calculated<Boolean>:: If true, the default value is calculated by the recipe and cannot be displayed.
-      #   type<String>:: "string" or "array" - default is "string"  ("hash" is supported for backwards compatibility)
-      #   required<String>:: Whether this attr is 'required', 'recommended' or 'optional' - default 'optional' (true/false values also supported for backwards compatibility)
-      #   recipes<Array>:: An array of recipes which need this attr set.
-      #   default<String>,<Array>,<Hash>:: The default value
-      #
-      # === Parameters
-      # name<String>:: The name of the attribute ('foo', or 'apache2/log_dir')
-      # options<Hash>:: The description of the options
-      #
-      # === Returns
-      # options<Hash>:: Returns the current options hash
-      def attribute(name, options)
-        validate(
-          options,
-          {
-            display_name: { kind_of: String },
-            description: { kind_of: String },
-            choice: { kind_of: [ Array ], default: [] },
-            calculated: { equal_to: [ true, false ], default: false },
-            type: { equal_to: %w{string array hash symbol boolean numeric}, default: "string" },
-            required: { equal_to: [ "required", "recommended", "optional", true, false ], default: "optional" },
-            recipes: { kind_of: [ Array ], default: [] },
-            default: { kind_of: [ String, Array, Hash, Symbol, Numeric, TrueClass, FalseClass ] },
-            source_url: { kind_of: String },
-            issues_url: { kind_of: String },
-            privacy: { kind_of: [ TrueClass, FalseClass ] },
-          }
-        )
-        options[:required] = remap_required_attribute(options[:required]) unless options[:required].nil?
-        validate_choice_array(options)
-        validate_calculated_default_rule(options)
-        validate_choice_default_rule(options)
-
-        @attributes[name] = options
-        @attributes[name]
-      end
-
       # Convert an Array of Gem::Dependency objects (chef_version/ohai_version) to an Array.
       #
       # Gem::Dependencey#to_s is not useful, and there is no #to_json defined on it or its component
@@ -475,7 +426,6 @@ class Chef
           PLATFORMS => platforms,
           DEPENDENCIES => dependencies,
           PROVIDING => providing,
-          ATTRIBUTES => attributes,
           RECIPES => recipes,
           VERSION => version,
           SOURCE_URL => source_url,
@@ -509,7 +459,6 @@ class Chef
         @platforms                    = o[PLATFORMS] if o.key?(PLATFORMS)
         @dependencies                 = handle_deprecated_constraints(o[DEPENDENCIES]) if o.key?(DEPENDENCIES)
         @providing                    = o[PROVIDING] if o.key?(PROVIDING)
-        @attributes                   = o[ATTRIBUTES] if o.key?(ATTRIBUTES)
         @recipes                      = o[RECIPES] if o.key?(RECIPES)
         @version                      = o[VERSION] if o.key?(VERSION)
         @source_url                   = o[SOURCE_URL] if o.key?(SOURCE_URL)
diff --git a/spec/integration/knife/cookbook_show_spec.rb b/spec/integration/knife/cookbook_show_spec.rb
index 7b894efe30..8ebd5403ae 100644
--- a/spec/integration/knife/cookbook_show_spec.rb
+++ b/spec/integration/knife/cookbook_show_spec.rb
@@ -40,7 +40,6 @@ describe "knife cookbook show", :workstation do
         cookbook_name: x
         frozen?:       false
         metadata:
-          attributes:
           chef_versions:
           dependencies:
           description:      
@@ -87,7 +86,6 @@ describe "knife cookbook show", :workstation do
 
     it "knife cookbook show x 1.0.0 metadata shows the metadata" do
       knife("cookbook show x 1.0.0 metadata").should_succeed <<~EOM
-        attributes:
         chef_versions:
         dependencies:
         description:      
diff --git a/spec/unit/cookbook/metadata_spec.rb b/spec/unit/cookbook/metadata_spec.rb
index 0f85d82734..8d65efbd6a 100644
--- a/spec/unit/cookbook/metadata_spec.rb
+++ b/spec/unit/cookbook/metadata_spec.rb
@@ -28,9 +28,8 @@ describe Chef::Cookbook::Metadata do
     before do
       @fields = [ :name, :description, :long_description, :maintainer,
                   :maintainer_email, :license, :platforms, :dependencies,
-                  :providing, :attributes, :recipes, :version,
-                  :source_url, :issues_url, :privacy, :ohai_versions, :chef_versions,
-                  :gems ]
+                  :providing, :recipes, :version, :source_url, :issues_url,
+                  :privacy, :ohai_versions, :chef_versions, :gems ]
     end
 
     it "does not depend on object identity for equality" do
@@ -113,10 +112,6 @@ describe Chef::Cookbook::Metadata do
       expect(metadata.dependencies).to eq(Mash.new)
     end
 
-    it "has an empty attributes list" do
-      expect(metadata.attributes).to eq(Mash.new)
-    end
-
     it "has an empty recipes list" do
       expect(metadata.recipes).to eq(Mash.new)
     end
@@ -415,289 +410,6 @@ describe Chef::Cookbook::Metadata do
     end
   end
 
-  describe "cookbook attributes" do
-    it "should allow you set an attributes metadata" do
-      attrs = {
-        "display_name" => "MySQL Databases",
-        "description" => "Description of MySQL",
-        "choice" => %w{dedicated shared},
-        "calculated" => false,
-        "type" => "string",
-        "required" => "recommended",
-        "recipes" => [ "mysql::server", "mysql::master" ],
-        "default" => [ ],
-        "source_url" => "http://example.com",
-        "issues_url" => "http://example.com/issues",
-        "privacy" => true,
-      }
-      expect(metadata.attribute("/db/mysql/databases", attrs)).to eq(attrs)
-    end
-
-    it "should not accept anything but a string for display_name" do
-      expect do
-        metadata.attribute("db/mysql/databases", display_name: "foo")
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", display_name: Hash.new)
-      end.to raise_error(ArgumentError)
-    end
-
-    it "should not accept anything but a string for the description" do
-      expect do
-        metadata.attribute("db/mysql/databases", description: "foo")
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", description: Hash.new)
-      end.to raise_error(ArgumentError)
-    end
-
-    it "should not accept anything but a string for the source_url" do
-      expect do
-        metadata.attribute("db/mysql/databases", source_url: "foo")
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", source_url: Hash.new)
-      end.to raise_error(ArgumentError)
-    end
-
-    it "should not accept anything but a string for the issues_url" do
-      expect do
-        metadata.attribute("db/mysql/databases", issues_url: "foo")
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", issues_url: Hash.new)
-      end.to raise_error(ArgumentError)
-    end
-
-    it "should not accept anything but true or false for the privacy flag" do
-      expect do
-        metadata.attribute("db/mysql/databases", privacy: true)
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", privacy: false)
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", privacy: "true")
-      end.to raise_error(ArgumentError)
-    end
-
-    it "should not accept anything but an array of strings for choice" do
-      expect do
-        metadata.attribute("db/mysql/databases", choice: %w{dedicated shared})
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", choice: [10, "shared"])
-      end.to raise_error(ArgumentError)
-      expect do
-        metadata.attribute("db/mysql/databases", choice: Hash.new)
-      end.to raise_error(ArgumentError)
-    end
-
-    it "should set choice to empty array by default" do
-      metadata.attribute("db/mysql/databases", {})
-      expect(metadata.attributes["db/mysql/databases"][:choice]).to eq([])
-    end
-
-    it "should let calculated be true or false" do
-      expect do
-        metadata.attribute("db/mysql/databases", calculated: true)
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", calculated: false)
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", calculated: Hash.new)
-      end.to raise_error(ArgumentError)
-    end
-
-    it "should set calculated to false by default" do
-      metadata.attribute("db/mysql/databases", {})
-      expect(metadata.attributes["db/mysql/databases"][:calculated]).to eq(false)
-    end
-
-    it "accepts String for the attribute type" do
-      expect do
-        metadata.attribute("db/mysql/databases", type: "string")
-      end.not_to raise_error
-    end
-
-    it "accepts Array for the attribute type" do
-      expect do
-        metadata.attribute("db/mysql/databases", type: "array")
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", type: Array.new)
-      end.to raise_error(ArgumentError)
-    end
-
-    it "accepts symbol for the attribute type" do
-      expect do
-        metadata.attribute("db/mysql/databases", type: "symbol")
-      end.not_to raise_error
-    end
-
-    it "should let type be hash (backwards compatibility only)" do
-      expect do
-        metadata.attribute("db/mysql/databases", type: "hash")
-      end.not_to raise_error
-    end
-
-    it "should let required be required, recommended or optional" do
-      expect do
-        metadata.attribute("db/mysql/databases", required: "required")
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", required: "recommended")
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", required: "optional")
-      end.not_to raise_error
-    end
-
-    it "should convert required true to required" do
-      expect do
-        metadata.attribute("db/mysql/databases", required: true)
-      end.not_to raise_error
-      # attrib = metadata.attributes["db/mysql/databases"][:required].should == "required"
-    end
-
-    it "should convert required false to optional" do
-      expect do
-        metadata.attribute("db/mysql/databases", required: false)
-      end.not_to raise_error
-      # attrib = metadata.attributes["db/mysql/databases"][:required].should == "optional"
-    end
-
-    it "should set required to 'optional' by default" do
-      metadata.attribute("db/mysql/databases", {})
-      expect(metadata.attributes["db/mysql/databases"][:required]).to eq("optional")
-    end
-
-    it "should make sure recipes is an array" do
-      expect do
-        metadata.attribute("db/mysql/databases", recipes: [])
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", required: Hash.new)
-      end.to raise_error(ArgumentError)
-    end
-
-    it "should set recipes to an empty array by default" do
-      metadata.attribute("db/mysql/databases", {})
-      expect(metadata.attributes["db/mysql/databases"][:recipes]).to eq([])
-    end
-
-    it "should allow the default value to be a string, array, hash, boolean or numeric" do
-      expect do
-        metadata.attribute("db/mysql/databases", default: [])
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", default: {})
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", default: "alice in chains")
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", default: 1337)
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", default: true)
-      end.not_to raise_error
-      expect do
-        metadata.attribute("db/mysql/databases", required: :not_gonna_do_it)
-      end.to raise_error(ArgumentError)
-    end
-
-    it "should limit the types allowed in the choice array" do
-      options = {
-        type: "string",
-        choice: %w{test1 test2},
-        default: "test1",
-      }
-      expect do
-        metadata.attribute("test_cookbook/test", options)
-      end.not_to raise_error
-
-      options = {
-        type: "boolean",
-        choice: [ true, false ],
-        default: true,
-      }
-      expect do
-        metadata.attribute("test_cookbook/test", options)
-      end.not_to raise_error
-
-      options = {
-        type: "numeric",
-        choice: [ 1337, 420 ],
-        default: 1337,
-      }
-      expect do
-        metadata.attribute("test_cookbook/test", options)
-      end.not_to raise_error
-
-      options = {
-        type: "numeric",
-        choice: [ true, "false" ],
-        default: false,
-      }
-      expect do
-        metadata.attribute("test_cookbook/test", options)
-      end.to raise_error(Chef::Exceptions::ValidationFailed)
-    end
-
-    it "should error if default used with calculated" do
-      expect do
-        attrs = {
-          calculated: true,
-          default: [ "I thought you said calculated" ],
-        }
-        metadata.attribute("db/mysql/databases", attrs)
-      end.to raise_error(ArgumentError)
-      expect do
-        attrs = {
-          calculated: true,
-          default: "I thought you said calculated",
-        }
-        metadata.attribute("db/mysql/databases", attrs)
-      end.to raise_error(ArgumentError)
-    end
-
-    it "should allow a default that is a choice" do
-      expect do
-        attrs = {
-          choice: %w{a b c},
-          default: "b",
-        }
-        metadata.attribute("db/mysql/databases", attrs)
-      end.not_to raise_error
-      expect do
-        attrs = {
-          choice: %w{a b c d e},
-          default: %w{b d},
-        }
-        metadata.attribute("db/mysql/databases", attrs)
-      end.not_to raise_error
-    end
-
-    it "should error if default is not a choice" do
-      expect do
-        attrs = {
-          choice: %w{a b c},
-          default: "d",
-        }
-        metadata.attribute("db/mysql/databases", attrs)
-      end.to raise_error(ArgumentError)
-      expect do
-        attrs = {
-          choice: %w{a b c d e},
-          default: %w{b z},
-        }
-        metadata.attribute("db/mysql/databases", attrs)
-      end.to raise_error(ArgumentError)
-    end
-  end
-
   describe "recipes" do
     let(:cookbook) do
       c = Chef::CookbookVersion.new("test_cookbook")
@@ -742,8 +454,6 @@ describe Chef::Cookbook::Metadata do
       metadata.depends "bobotclown", "= 1.1"
       metadata.provides "foo(:bar, :baz)"
       metadata.recipe "test_cookbook::enlighten", "is your buddy"
-      metadata.attribute "bizspark/has_login",
-        display_name: "You have nothing"
       metadata.version "1.2.3"
       metadata.gem "foo", "~> 1.2"
       metadata.gem "bar", ">= 2.2", "< 4.0"
@@ -776,7 +486,6 @@ describe Chef::Cookbook::Metadata do
         platforms
         dependencies
         providing
-        attributes
         recipes
         version
         source_url
@@ -817,7 +526,6 @@ describe Chef::Cookbook::Metadata do
         platforms
         dependencies
         providing
-        attributes
         recipes
         version
         source_url
diff --git a/spec/unit/knife/cookbook_show_spec.rb b/spec/unit/knife/cookbook_show_spec.rb
index 6642e0690d..6054f8c6b6 100644
--- a/spec/unit/knife/cookbook_show_spec.rb
+++ b/spec/unit/knife/cookbook_show_spec.rb
@@ -116,7 +116,6 @@ describe Chef::Knife::CookbookShow do
             "platforms" => {},
             "dependencies" => {},
             "providing" => {},
-            "attributes" => {},
             "recipes" => {},
             "version" => "0.0.0",
             "source_url" => "",