Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ export BAT_INFRASTRUCTURE=

# Run tests with --fail-fast and skip cleanup in case of failure (optional)
export BAT_DEBUG_MODE=

# Provide an alternate directory where Bat::CloudConfig#load_template(cpi) should find "cloud_config_${cpi}.yml.erb" files
export BAT_CC_TEMPLATES_DIR=
```

#### Environment variables for the BOSH v2 cli
Expand Down
18 changes: 16 additions & 2 deletions lib/bat/cloud_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ module Bosh; end # Ugly hack

module Bat
class CloudConfig
def initialize(spec)
DEFAULT_TEMPLATES_DIR = File.expand_path("../../../templates/", __FILE__)

def initialize(env, spec)
@env = env
@spec = spec
generate_cloud_config(spec)
end
Expand Down Expand Up @@ -53,7 +56,8 @@ def store_cloud_config(content)
end

def load_template(cpi)
template = File.expand_path("../../../templates/cloud_config_#{cpi}.yml.erb", __FILE__)
template = File.join(cc_templates_dir, "cloud_config_#{cpi}.yml.erb")

File.read(template)
end

Expand All @@ -64,5 +68,15 @@ def tempfile(name)
def to_s
"#{name}"
end

def cc_templates_dir
if @env.cc_templates_dir &&
@env.cc_templates_dir != "" &&
Dir.exist?(@env.cc_templates_dir)
@env.cc_templates_dir
else
DEFAULT_TEMPLATES_DIR
end
end
Comment on lines +71 to +80

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid silent fallbacks for explicit configurations.

If BAT_CC_TEMPLATES_DIR is explicitly set to a non-existent path, Dir.exist? evaluates to false and the code silently falls back to DEFAULT_TEMPLATES_DIR. This can lead to confusing test behavior where the default templates are used instead of custom ones, without warning the user.

Removing the Dir.exist? check allows File.read downstream to fail loudly with an Errno::ENOENT error indicating the exact path it attempted to read from, making misconfigurations much easier to debug.

♻️ Proposed fix
    def cc_templates_dir
-      if `@env.cc_templates_dir` &&
-         `@env.cc_templates_dir` != "" &&
-         Dir.exist?(`@env.cc_templates_dir`)
+      if `@env.cc_templates_dir` && !`@env.cc_templates_dir.empty`?
        `@env.cc_templates_dir`
      else
        DEFAULT_TEMPLATES_DIR
      end
    end
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def cc_templates_dir
if @env.cc_templates_dir &&
@env.cc_templates_dir != "" &&
Dir.exist?(@env.cc_templates_dir)
@env.cc_templates_dir
else
DEFAULT_TEMPLATES_DIR
end
end
def cc_templates_dir
if `@env.cc_templates_dir` && !`@env.cc_templates_dir.empty`?
`@env.cc_templates_dir`
else
DEFAULT_TEMPLATES_DIR
end
end
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/bat/cloud_config.rb` around lines 71 - 80, Update cc_templates_dir to
stop checking Dir.exist? for an explicitly configured `@env.cc_templates_dir`.
Preserve the existing non-empty configuration check, return the configured path
as-is, and retain DEFAULT_TEMPLATES_DIR only when no path is configured.

end
end
1 change: 1 addition & 0 deletions lib/bat/env.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class Env

OPTIONAL_VARS = {
debug_mode: 'BAT_DEBUG_MODE',
cc_templates_dir: 'BAT_CC_TEMPLATES_DIR',
}.freeze
Comment on lines 11 to 14

def self.from_env
Expand Down
8 changes: 4 additions & 4 deletions lib/bat/requirements.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ module Bat
class Requirements
include RSpec::Matchers

def initialize(stemcell_path, bosh_runner, spec_state, logger)
@stemcell_path = stemcell_path
def initialize(env, bosh_runner, spec_state, logger)
@env = env
@bosh_runner = bosh_runner
@spec_state = spec_state
@logger = logger
Comment thread
aramprice marked this conversation as resolved.
end

def stemcell
@stemcell ||= Bat::Stemcell.from_path(@stemcell_path)
@stemcell ||= Bat::Stemcell.from_path(@env.stemcell_path)
end

def release
Expand Down Expand Up @@ -82,7 +82,7 @@ def only_scan_and_fix_processing?
end

def update_cloud_config(deployment_spec)
cloud_config = Bat::CloudConfig.new(deployment_spec)
cloud_config = Bat::CloudConfig.new(@env, deployment_spec)

update_cc_output = @bosh_runner.bosh_safe("update-cloud-config #{cloud_config.to_path}")
puts update_cc_output
Expand Down
6 changes: 5 additions & 1 deletion spec/bat/requirements_spec.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'spec_helper'
require 'bat/env'
require 'bat/requirements'
require 'bat/bosh_runner'
require 'bat/spec_state'
Expand All @@ -7,10 +8,13 @@
require 'common/exec'

describe Bat::Requirements do
subject(:requirements) { described_class.new("/some-stemcell/path", bosh_runner, spec_state, logger) }
subject(:requirements) { described_class.new(bat_env, bosh_runner, spec_state, logger) }

let(:bosh_runner) { Bat::BoshRunner.new("bosh", logger) }
let(:logger) { Logger.new('/dev/null') }
let(:spec_state) { Bat::SpecState.new(false) }
let(:stemcell_path) { "/some-stemcell/path" }
let(:bat_env) { instance_double(Bat::Env, stemcell_path: stemcell_path) }

describe "#tasks_processing?" do
let(:tasks_result) { Bosh::Exec::Result.new('bosh tasks', tasks_json, 0) }
Expand Down
2 changes: 1 addition & 1 deletion spec/system/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class OnlyScanAndFixProcessing < StandardError;end
env = Bat::Env.from_env
spec_state = Bat::SpecState.new(env.debug_mode)
bosh_runner = Bat::BoshRunner.new(ENV['BAT_BOSH_CLI'], logger)
requirements = Bat::Requirements.new(env.stemcell_path, bosh_runner, spec_state, logger)
requirements = Bat::Requirements.new(env, bosh_runner, spec_state, logger)

RSpec.configure do |config|
config.include(Bat::BoshHelper)
Expand Down