Skip to content

Commit

Permalink
Terraform: remove legacy terraform feature flag
Browse files Browse the repository at this point in the history
The legacy terraform parser is currently not used in production, this
removes the feature flag to use it in local dev, that we left behind to
make it easier to revert.

Once we're confident we won't need to do that, we can remove the old
implementation by merging this in.
  • Loading branch information
jurre committed May 14, 2021
1 parent 2476379 commit 996d4a8
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 135 deletions.
1 change: 0 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ jobs:
- { path: omnibus, name: omnibus }
- { path: python, name: python }
- { path: terraform, name: terraform }
- { path: terraform, name: terraform-hcl2 }
steps:
- name: Checkout code
uses: actions/checkout@v2
Expand Down
6 changes: 0 additions & 6 deletions bin/dry-run.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@
security_updates_only: false,
ignore_conditions: [],
pull_request: false,
legacy_terraform: false
}

unless ENV["LOCAL_GITHUB_ACCESS_TOKEN"].to_s.strip.empty?
Expand Down Expand Up @@ -147,10 +146,6 @@
$options[:ignore_conditions] = JSON.parse(ENV["IGNORE_CONDITIONS"])
end

unless ENV["LEGACY_TERRAFORM"].to_s.strip.empty?
$options[:legacy_terraform] = true
end

# rubocop:disable Metrics/BlockLength
option_parse = OptionParser.new do |opts|
opts.banner = "usage: ruby bin/dry-run.rb [OPTIONS] PACKAGE_MANAGER REPO"
Expand Down Expand Up @@ -498,7 +493,6 @@ def handle_dependabot_error(error:, dependency:)
source: $source,
credentials: $options[:credentials],
reject_external_code: $options[:reject_external_code],
options: {legacy_terraform: $options[:legacy_terraform]}
)

dependencies = cached_read("dependencies") { parser.parse }
Expand Down
7 changes: 0 additions & 7 deletions terraform/helpers/build
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,6 @@ fi

os="$(uname -s | tr '[:upper:]' '[:lower:]')"

json2hcl_checksum="d124ed13f3538c465fcab19e6015d311d3cd56f7dc2db7609b6e72fec666482d"
json2hcl_url="https://github.com/kvz/json2hcl/releases/download/v0.0.6/json2hcl_v0.0.6_${os}_amd64"
json2hcl_path="$install_dir/bin/json2hcl"
wget -O "$json2hcl_path" "$json2hcl_url"
echo "$json2hcl_checksum $json2hcl_path" | sha256sum -c
chmod +x "$install_dir/bin/json2hcl"

hcl2json_checksum="24068f1e25a34d8f8ca763f34fce11527472891bfa834d1504f665855021d5d4"
hcl2json_url="https://github.com/tmccombs/hcl2json/releases/download/v0.3.3/hcl2json_${os}_amd64"
hcl2json_path="$install_dir/bin/hcl2json"
Expand Down
76 changes: 21 additions & 55 deletions terraform/lib/dependabot/terraform/file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -215,56 +215,6 @@ def source_type(source_string)
end
# rubocop:enable Metrics/PerceivedComplexity

def parsed_file_hcl2(file)
SharedHelpers.in_a_temporary_directory do
File.write("tmp.tf", file.content)

command = "#{terraform_hcl2_parser_path} < tmp.tf"
start = Time.now
stdout, stderr, process = Open3.capture3(command)
time_taken = Time.now - start

unless process.success?
raise SharedHelpers::HelperSubprocessFailed.new(
message: stderr,
error_context: {
command: command,
time_taken: time_taken,
process_exit_value: process.to_s
}
)
end

JSON.parse(stdout)
end
end

def parsed_file_hcl1(file)
SharedHelpers.in_a_temporary_directory do
File.write("tmp.tf", file.content)

command = "#{terraform_parser_path} -reverse < tmp.tf"
start = Time.now
stdout, stderr, process = Open3.capture3(command)
time_taken = Time.now - start

unless process.success?
raise SharedHelpers::HelperSubprocessFailed.new(
message: stderr,
error_context: {
command: command,
time_taken: time_taken,
process_exit_value: process.to_s
}
)
end

json = JSON.parse(stdout)
json["module"] = json.fetch("module", []).inject({}) { |memo, item| memo.merge(item) }
json
end
end

# == Returns:
# A Hash representing each module found in the specified file
#
Expand All @@ -289,12 +239,28 @@ def parsed_file_hcl1(file)
# }
def parsed_file(file)
@parsed_buildfile ||= {}
@parsed_buildfile[file.name] ||=
if options[:legacy_terraform]
parsed_file_hcl1(file)
else
parsed_file_hcl2(file)
@parsed_buildfile[file.name] ||= SharedHelpers.in_a_temporary_directory do
File.write("tmp.tf", file.content)

command = "#{terraform_hcl2_parser_path} < tmp.tf"
start = Time.now
stdout, stderr, process = Open3.capture3(command)
time_taken = Time.now - start

unless process.success?
raise SharedHelpers::HelperSubprocessFailed.new(
message: stderr,
error_context: {
command: command,
time_taken: time_taken,
process_exit_value: process.to_s
}
)
end

JSON.parse(stdout)
end

rescue SharedHelpers::HelperSubprocessFailed => e
msg = e.message.strip
raise Dependabot::DependencyFileNotParseable.new(file.path, msg)
Expand Down
47 changes: 3 additions & 44 deletions terraform/spec/dependabot/terraform/file_parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@
subject(:parser) do
described_class.new(
dependency_files: files,
source: source,
options: {
legacy_terraform: PackageManagerHelper.use_terraform_hcl1?
}
source: source
)
end

Expand All @@ -38,14 +35,7 @@
context "with an unparseable source" do
let(:files) { project_dependency_files("unparseable") }

it "raises an error for hcl1", :hcl1_only do
expect { subject }.to raise_error(Dependabot::DependencyFileNotParseable) do |boom|
expect(boom.file_path).to eq("/main.tf")
expect(boom.message).to eq("unable to parse HCL: object expected closing RBRACE got: EOF")
end
end

it "raises an error for hcl2", :hcl2_only do
it "raises an error" do
expect { subject }.to raise_error(Dependabot::DependencyFileNotParseable) do |boom|
expect(boom.message).to eq(
"Failed to convert file: parse config: [:18,1-1: Argument or block definition required; " \
Expand Down Expand Up @@ -239,30 +229,7 @@
end
end

context "with a terragrunt file", :hcl1_only do
let(:files) { project_dependency_files("terragrunt") }

specify { expect(subject.length).to eq(1) }
specify { expect(subject).to all(be_a(Dependabot::Dependency)) }

it "has the right details for the first dependency" do
expect(subject[0].name).to eq("gruntwork-io/modules-example")
expect(subject[0].version).to eq("0.0.2")
expect(subject[0].requirements).to eq([{
requirement: nil,
groups: [],
file: "main.tfvars",
source: {
type: "git",
url: "git@github.com:gruntwork-io/modules-example.git",
branch: nil,
ref: "v0.0.2"
}
}])
end
end

context "hcl2 files", :hcl2_only do
context "hcl2 files" do
let(:files) { project_dependency_files("hcl2") }

it "has the right source for the dependency" do
Expand Down Expand Up @@ -533,14 +500,6 @@
end
end

context "hcl2 files using the old parser", :hcl1_only do
let(:files) { project_dependency_files("hcl2") }

it "fails to parse hcl2 files without the flag set" do
expect { subject }.to raise_error(Dependabot::DependencyFileNotParseable)
end
end

context "terraform.lock.hcl files" do
let(:files) { project_dependency_files("terraform_lock_only") }

Expand Down
22 changes: 0 additions & 22 deletions terraform/spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,6 @@ def require_common_spec(path)

require "#{common_dir}/spec/spec_helper.rb"

module PackageManagerHelper
def self.use_terraform_hcl1?
ENV["LEGACY_TERRAFORM"]
end

def self.use_terraform_hcl2?
!use_terraform_hcl1?
end
end

RSpec.configure do |config|
config.around do |example|
if PackageManagerHelper.use_terraform_hcl2? && example.metadata[:hcl1_only]
example.skip
elsif PackageManagerHelper.use_terraform_hcl1? && example.metadata[:hcl2_only]
example.skip
else
example.run
end
end
end

if ENV["COVERAGE"]
# TODO: Bring branch coverage up
SimpleCov.minimum_coverage line: 80, branch: 55
Expand Down

0 comments on commit 996d4a8

Please sign in to comment.