Skip to content

Commit

Permalink
coderabbitai review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
zzaakiirr committed Oct 21, 2024
1 parent ed31386 commit d41fb62
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 67 deletions.
23 changes: 11 additions & 12 deletions lib/core/terraform_config/generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,17 @@ def policy_config_params
.merge(gvc: gvc, target_links: policy_target_links, bindings: policy_bindings)
end

def volumeset_config_params # rubocop:disable Metrics/MethodLength
template
.slice(:name, :description, :tags)
.merge(
gvc: gvc,
initial_capacity: template.dig(:spec, :initial_capacity),
performance_class: template.dig(:spec, :performance_class),
file_system_type: template.dig(:spec, :file_system_type),
storage_class_suffix: template.dig(:spec, :storage_class_suffix),
snapshots: template.dig(:spec, :snapshots),
autoscaling: template.dig(:spec, :autoscaling)
)
def volumeset_config_params
specs = %i[
initial_capacity
performance_class
file_system_type
storage_class_suffix
snapshots
autoscaling
].to_h { |key| [key, template.dig(:spec, key)] }

template.slice(:name, :description, :tags).merge(gvc: gvc).merge(specs)
end

# GVC name matches application name
Expand Down
24 changes: 16 additions & 8 deletions lib/core/terraform_config/volume_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ def validate_attributes!
end

def validate_initial_capacity!
return unless initial_capacity < MIN_CAPACITY
raise ArgumentError, "Initial capacity must be numeric" unless initial_capacity.is_a?(Numeric)
return if initial_capacity >= MIN_CAPACITY

raise ArgumentError,
"Initial capacity should be greater than or equal to #{MIN_CAPACITY}"
raise ArgumentError, "Initial capacity should be >= #{MIN_CAPACITY}"
end

def validate_performance_class!
Expand All @@ -72,8 +72,7 @@ def validate_performance_class!
def validate_file_system_type!
return if FILE_SYSTEM_TYPES.include?(file_system_type)

raise ArgumentError,
"Invalid file system type: #{file_system_type}. Choose from #{FILE_SYSTEM_TYPES.join(', ')}"
raise ArgumentError, "Invalid file system type: #{file_system_type}. Choose from #{FILE_SYSTEM_TYPES.join(', ')}"
end

def validate_autoscaling!
Expand All @@ -84,21 +83,30 @@ def validate_autoscaling!

def validate_max_capacity!
max_capacity = autoscaling.fetch(:max_capacity, nil)
return if max_capacity.nil? || max_capacity >= MIN_CAPACITY
return if max_capacity.nil?

raise ArgumentError, "autoscaling.max_capacity must be numeric" unless max_capacity.is_a?(Numeric)
return if max_capacity >= MIN_CAPACITY

raise ArgumentError, "autoscaling.max_capacity should be >= #{MIN_CAPACITY}"
end

def validate_min_free_percentage!
min_free_percentage = autoscaling.fetch(:min_free_percentage, nil)
return if min_free_percentage.nil? || min_free_percentage.between?(1, 100)
return if min_free_percentage.nil?

raise ArgumentError, "autoscaling.min_free_percentage must be numeric" unless min_free_percentage.is_a?(Numeric)
return if min_free_percentage.between?(1, 100)

raise ArgumentError, "autoscaling.min_free_percentage should be between 1 and 100"
end

def validate_scaling_factor!
scaling_factor = autoscaling.fetch(:scaling_factor, nil)
return if scaling_factor.nil? || scaling_factor >= MIN_SCALING_FACTOR
return if scaling_factor.nil?

raise ArgumentError, "autoscaling.scaling_factor must be numeric" unless scaling_factor.is_a?(Numeric)
return if scaling_factor >= MIN_SCALING_FACTOR

raise ArgumentError, "autoscaling.scaling_factor should be >= #{MIN_SCALING_FACTOR}"
end
Expand Down
78 changes: 31 additions & 47 deletions spec/core/terraform_config/volume_set_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,73 +99,57 @@
end

describe "validations" do
context "with invalid initial_capacity" do
it "raises an error" do
expect { described_class.new(**options.merge(initial_capacity: 5)) }.to raise_error(
ArgumentError, "Initial capacity should be greater than or equal to 10"
)
shared_examples "an invalid parameter" do |param, value, err_msg|
it "raises an error for invalid #{param}" do
expect { described_class.new(**options.merge(param => value)) }.to raise_error(ArgumentError, err_msg)
end
end

context "with invalid performance_class" do
it "raises an error" do
expect { described_class.new(**options.merge(performance_class: "invalid")) }.to raise_error(
ArgumentError, "Invalid performance class: invalid. Choose from general-purpose-ssd, high-throughput-ssd"
)
end
end
include_examples "an invalid parameter", :initial_capacity, 5, "Initial capacity should be >= 10"
include_examples "an invalid parameter", :initial_capacity, "10", "Initial capacity must be numeric"

include_examples "an invalid parameter",
:performance_class, "invalid",
"Invalid performance class: invalid. Choose from general-purpose-ssd, high-throughput-ssd"

context "with invalid file_system_type" do
include_examples "an invalid parameter",
:file_system_type, "invalid",
"Invalid file system type: invalid. Choose from xfs, ext4"
end

describe "autoscaling validations" do
shared_examples "invalid autoscaling parameter" do |autoscaling_options, err_msg|
it "raises an error" do
expect { described_class.new(**options.merge(file_system_type: "invalid")) }.to raise_error(
ArgumentError, "Invalid file system type: invalid. Choose from xfs, ext4"
expect { described_class.new(**options.merge(autoscaling: autoscaling_options)) }.to raise_error(
ArgumentError, err_msg
)
end
end
end

describe "autoscaling validations" do
context "with invalid max_capacity" do
let(:invalid_autoscaling) do
options.merge(autoscaling: { max_capacity: 5 })
end
include_examples "invalid autoscaling parameter", { max_capacity: 5 }, "autoscaling.max_capacity should be >= 10"

it "raises an error" do
expect { described_class.new(**invalid_autoscaling) }.to raise_error(
ArgumentError, "autoscaling.max_capacity should be >= 10"
)
end
include_examples "invalid autoscaling parameter", { max_capacity: "100" },
"autoscaling.max_capacity must be numeric"
end

context "with invalid min_free_percentage" do
let(:invalid_autoscaling) do
options.merge(autoscaling: { min_free_percentage: 0 })
end
include_examples "invalid autoscaling parameter", { min_free_percentage: 0 },
"autoscaling.min_free_percentage should be between 1 and 100"

it "raises an error for value below 1" do
expect { described_class.new(**invalid_autoscaling) }.to raise_error(
ArgumentError, "autoscaling.min_free_percentage should be between 1 and 100"
)
end
include_examples "invalid autoscaling parameter", { min_free_percentage: 101 },
"autoscaling.min_free_percentage should be between 1 and 100"

it "raises an error for value above 100" do
invalid_autoscaling[:autoscaling][:min_free_percentage] = 101
expect { described_class.new(**invalid_autoscaling) }.to raise_error(
ArgumentError, "autoscaling.min_free_percentage should be between 1 and 100"
)
end
include_examples "invalid autoscaling parameter", { min_free_percentage: "50" },
"autoscaling.min_free_percentage must be numeric"
end

context "with invalid scaling_factor" do
let(:invalid_autoscaling) do
options.merge(autoscaling: { scaling_factor: 1.0 })
end
include_examples "invalid autoscaling parameter", { scaling_factor: 1.0 },
"autoscaling.scaling_factor should be >= 1.1"

it "raises an error" do
expect { described_class.new(**invalid_autoscaling) }.to raise_error(
ArgumentError, "autoscaling.scaling_factor should be >= 1.1"
)
end
include_examples "invalid autoscaling parameter", { scaling_factor: "1.5" },
"autoscaling.scaling_factor must be numeric"
end

context "with valid autoscaling values" do
Expand Down

0 comments on commit d41fb62

Please sign in to comment.