Removes nats user and password from hm / director

- they were not being used now that we have mutual TLS - the CPI will actually send the mbus URL for non-mutual TLS if the CPI has been configured with nats username and password - the tests have been modified to fix legacy agent support [#151420600](https://www.pivotaltracker.com/story/show/151420600) Signed-off-by: Jamil Shamy <[email protected]>
cloudfoundry · Oct 19, 2017 · b0da4b0 · b0da4b0
1 parent 67591f0
commit b0da4b0
Show file tree

Hide file tree

Showing 22 changed files with 112 additions and 105 deletions.
diff --git a/jobs/director/spec b/jobs/director/spec
@@ -199,11 +199,6 @@ properties:
     default: vcap
 
   # NATs
-  nats.user:
-    description: Username to connect to nats with
-    default: nats
-  nats.password:
-    description: Password to connect to nats with
   nats.address:
     description: Address of the nats server
   nats.port:

diff --git a/jobs/director/templates/director.yml.erb.erb b/jobs/director/templates/director.yml.erb.erb
@@ -12,7 +12,7 @@ params = {
     'level' => 'DEBUG',
     'file' =>  "/var/vcap/sys/log/director/<" + "%= ENV['COMPONENT'] %" + ">.debug.log"
   },
-  'mbus' => "nats://#{p('nats.user')}:#{p('nats.password')}@#{p('nats.address')}:#{p('nats.port')}",
+  'mbus' => "nats://#{p('nats.address')}:#{p('nats.port')}",
   'nats' => {
     'server_ca_path' => '/var/vcap/jobs/director/config/nats_server_ca.pem',
     'client_ca_certificate_path' => '/var/vcap/jobs/director/config/nats_client_ca_certificate.pem',
@@ -386,6 +386,7 @@ if_p('director.default_ssh_options.gateway_host',
 end
 
 cpi_job_name = p('director.cpi_job')
+
 params['cloud'] = {
     'provider' => {
       'name' => cpi_job_name,
@@ -395,12 +396,12 @@ params['cloud'] = {
 }
 
 params['cloud']['properties']['agent'] = {
- 'ntp' => p('ntp'),
- 'blobstore' => {'provider' => p('blobstore.provider'), 'options' => {} },
- 'mbus' => "nats://#{p('nats.user')}:#{p('nats.password')}@#{p(['agent.nats.address', 'nats.address'])}:#{p('nats.port')}"
+  'ntp' => p('ntp'),
+  'blobstore' => {'provider' => p('blobstore.provider'), 'options' => {} },
 }
 
 agent_blobstore_options =  params['cloud']['properties']['agent']['blobstore']['options']
+
 if p('blobstore.provider') == "s3"
   agent_blobstore_options['bucket_name'] = p('blobstore.bucket_name')
   agent_blobstore_options['credentials_source'] = p(['agent.blobstore.credentials_source', 'blobstore.credentials_source'], 'static')

diff --git a/jobs/health_monitor/spec b/jobs/health_monitor/spec
@@ -67,10 +67,6 @@ properties:
   nats.port:
     description: Port of the NATS message bus port to connect to
     default: 4222
-  nats.user:
-    description: User for the NATS message bus connection
-  nats.password:
-    description: Password for NATS message bus connection
   nats.tls.ca:
     description: 'CA cert to trust when communicating with NATS server'
   nats.tls.health_monitor.certificate:

diff --git a/jobs/health_monitor/templates/health_monitor.yml.erb b/jobs/health_monitor/templates/health_monitor.yml.erb
@@ -16,8 +16,6 @@ params = {
   },
   'mbus' => {
     'endpoint' => "nats://#{p('nats.address')}:#{p('nats.port')}",
-    'user' => p('nats.user'),
-    'password' => p('nats.password'),
     'server_ca_path' => '/var/vcap/jobs/health_monitor/config/nats_server_ca.pem',
     'client_certificate_path' => '/var/vcap/jobs/health_monitor/config/nats_client_certificate.pem',
     'client_private_key_path' => '/var/vcap/jobs/health_monitor/config/nats_client_private_key'

diff --git a/spec/director.yml.erb.erb_spec.rb b/spec/director.yml.erb.erb_spec.rb
@@ -24,8 +24,6 @@
         'provider' => 'dav',
       },
       'nats' => {
-        'user' => 'nats',
-        'password' => '1a0312a24c0a0',
         'address' => '10.10.0.7',
         'port' => 4222
       },

diff --git a/spec/director_templates_spec.rb b/spec/director_templates_spec.rb
@@ -2,7 +2,7 @@
 require 'bosh/template/evaluation_context'
 require_relative './template_example_group'
 
-describe 'director tempaltes' do
+describe 'director templates' do
  describe 'director' do
    describe 'nats_client_certificate.pem.erb' do
      it_should_behave_like 'a rendered file' do
@@ -83,4 +83,4 @@
    end
  end
 
-end
+end
diff --git a/spec/health_monitor_templates_spec.rb b/spec/health_monitor_templates_spec.rb
@@ -45,8 +45,6 @@
         'nats' => {
           'address' => '0.0.0.0',
           'port' => 4222,
-          'user' => 'my-user',
-          'password' => 'my-password',
         },
         'director' => {
           'address' => '0.0.0.0',
@@ -67,8 +65,6 @@
     it 'renders' do
       expect(parsed_yaml['http']['port']).to eq(8081)
       expect(parsed_yaml['mbus']['endpoint']).to eq('nats://0.0.0.0:4222')
-      expect(parsed_yaml['mbus']['user']).to eq('my-user')
-      expect(parsed_yaml['mbus']['password']).to eq('my-password')
       expect(parsed_yaml['mbus']['server_ca_path']).to eq('/var/vcap/jobs/health_monitor/config/nats_server_ca.pem')
       expect(parsed_yaml['mbus']['client_certificate_path']).to eq('/var/vcap/jobs/health_monitor/config/nats_client_certificate.pem')
       expect(parsed_yaml['mbus']['client_private_key_path']).to eq('/var/vcap/jobs/health_monitor/config/nats_client_private_key')

diff --git a/src/bosh-dev/assets/sandbox/cpi_config.json.erb b/src/bosh-dev/assets/sandbox/cpi_config.json.erb
@@ -0,0 +1,12 @@
+{
+ "dir": "<%= cloud_storage_dir %>",
+ "nats":  "<%= nats_url %>",
+ "agent": {
+   "blobstore": {
+     "provider": "local",
+       "options": {
+         "blobstore_path": "<%= blobstore_storage_dir %>"
+      }
+    }
+  }
+}
diff --git a/src/bosh-dev/assets/sandbox/director_test.yml.erb b/src/bosh-dev/assets/sandbox/director_test.yml.erb
@@ -7,11 +7,7 @@ runtime:
 
 port: <%= director_ruby_port %>
 
-<% if nats_allow_legacy_clients %>
-mbus: nats://<%= nats_user %>:<%= nats_password %>@127.0.0.1:<%= nats_port %>
-<% else %>
 mbus: nats://127.0.0.1:<%= nats_port %>
-<% end %>
 
 logging:
   level: DEBUG
@@ -85,19 +81,10 @@ cloud:
     name: <%= external_cpi_config[:name] %>
     path: <%= external_cpi_config[:job_path] %>
   properties:
-<% if nats_allow_legacy_clients %>
-    nats: nats://<%= nats_user %>:<%= nats_password %>@localhost:<%= nats_port %>
-<% else %>
-    nats: nats://localhost:<%= nats_port %>
-<% end %>
     dir: <%= cloud_storage_dir %>
     agent:
       blobstore:
         <<: *director_blobstore
-      server:
-        host: 127.0.0.1
-        host: 127.0.0.1
-        password:
 
 user_management:
   provider: <%= user_authentication %>

diff --git a/src/bosh-dev/assets/sandbox/health_monitor.yml.erb b/src/bosh-dev/assets/sandbox/health_monitor.yml.erb
@@ -4,13 +4,6 @@ http:
 
 mbus:
   endpoint: nats://localhost:<%= nats_port %>
-<% if nats_allow_legacy_clients %>
-  user: <%= nats_user %>
-  password: <%= nats_password %>
-<% else %>
-  user:
-  password:
-<% end %>
   server_ca_path: <%= nats_certificate_paths['ca_path'] %>
   client_private_key_path: <%= nats_certificate_paths['clients']['health_monitor']['private_key_path'] %>
   client_certificate_path: <%= nats_certificate_paths['clients']['health_monitor']['certificate_path'] %>

diff --git a/src/bosh-dev/assets/sandbox/health_monitor_with_json_logging.yml.erb b/src/bosh-dev/assets/sandbox/health_monitor_with_json_logging.yml.erb
@@ -4,8 +4,6 @@ http:
 
 mbus:
   endpoint: nats://localhost:<%= nats_port %>
-  user:
-  password:
   server_ca_path: <%= nats_certificate_paths['ca_path'] %>
   client_private_key_path: <%= nats_certificate_paths['clients']['health_monitor']['private_key_path'] %>
   client_certificate_path: <%= nats_certificate_paths['clients']['health_monitor']['certificate_path'] %>
@@ -34,4 +32,4 @@ plugins:
       - alert
       - heartbeat
     options:
-      format: json
+      format: json
diff --git a/src/bosh-dev/assets/sandbox/health_monitor_without_resurrector.yml.erb b/src/bosh-dev/assets/sandbox/health_monitor_without_resurrector.yml.erb
@@ -4,8 +4,6 @@ http:
 
 mbus:
   endpoint: nats://localhost:<%= nats_port %>
-  user:
-  password:
   server_ca_path: <%= nats_certificate_paths['ca_path'] %>
   client_private_key_path: <%= nats_certificate_paths['clients']['health_monitor']['private_key_path'] %>
   client_certificate_path: <%= nats_certificate_paths['clients']['health_monitor']['certificate_path'] %>

diff --git a/src/bosh-dev/lib/bosh/dev/sandbox/director_config.rb b/src/bosh-dev/lib/bosh/dev/sandbox/director_config.rb
@@ -32,10 +32,7 @@ class DirectorConfig
       :nats_server_ca_path,
       :nats_client_ca_private_key_path,
       :nats_client_ca_certificate_path,
-      :nats_director_tls,
-      :nats_allow_legacy_clients,
-      :nats_user,
-      :nats_password
+      :nats_director_tls
 
     def initialize(attrs, port_provider)
       @director_name = 'TestDirector'
@@ -82,9 +79,6 @@ def initialize(attrs, port_provider)
       @nats_client_ca_private_key_path = attrs.fetch(:nats_client_ca_private_key_path)
       @nats_client_ca_certificate_path = attrs.fetch(:nats_client_ca_certificate_path)
       @nats_director_tls = attrs.fetch(:nats_director_tls)
-      @nats_allow_legacy_clients = attrs.fetch(:nats_allow_legacy_clients)
-      @nats_user = attrs.fetch(:nats_user)
-      @nats_password = attrs.fetch(:nats_password)
     end
 
     def render(template_path)

diff --git a/src/bosh-dev/lib/bosh/dev/sandbox/main.rb b/src/bosh-dev/lib/bosh/dev/sandbox/main.rb
@@ -34,6 +34,9 @@ class Main
     EXTERNAL_CPI = 'cpi'
     EXTERNAL_CPI_TEMPLATE = File.join(SANDBOX_ASSETS_DIR, 'cpi.erb')
 
+    EXTERNAL_CPI_CONFIG = 'cpi.json'
+    EXTERNAL_CPI_CONFIG_TEMPLATE = File.join(SANDBOX_ASSETS_DIR, 'cpi_config.json.erb')
+
     UPGRADE_SPEC_ASSETS_DIR = File.expand_path('spec/assets/upgrade', REPO_ROOT)
 
     attr_reader :name
@@ -56,7 +59,7 @@ class Main
     attr_reader :nats_log_path
     attr_reader :nats_host
 
-    attr_reader :nats_user, :nats_password, :nats_allow_legacy_clients
+    attr_reader :nats_url, :nats_user, :nats_password, :nats_allow_legacy_clients
     attr_reader :nats_needs_restart
 
     attr_accessor :trusted_certs
@@ -129,11 +132,23 @@ def initialize(db_opts, debug, test_env_number, logger)
 
       # Note that this is not the same object
       # as dummy cpi used inside bosh-director process
-      @cpi = Bosh::Clouds::Dummy.new({
-        'dir' => cloud_storage_dir,
-        'agent' => {'blobstore' => {}},
-        'nats' => "nats://localhost:#{nats_port}"}, {})
-      reconfigure({})
+      @cpi = Bosh::Clouds::Dummy.new(
+        {
+          'dir' => cloud_storage_dir,
+          'agent' => {
+            'blobstore' => {
+              'provider' => 'local',
+              'options' => {
+                'blobstore_path' => @blobstore_storage_dir,
+              },
+            }
+          },
+          'nats' => @nats_url,
+        },
+        {}
+      )
+
+      reconfigure
     end
 
     def agent_tmp_path
@@ -203,9 +218,6 @@ def director_config
         nats_client_ca_private_key_path: get_nats_client_ca_private_key_path,
         nats_client_ca_certificate_path: get_nats_client_ca_certificate_path,
         nats_director_tls: nats_certificate_paths['clients']['director'],
-        nats_allow_legacy_clients: @nats_allow_legacy_clients,
-        nats_user: @nats_user,
-        nats_password: @nats_password,
       }
       DirectorConfig.new(attributes, @port_provider)
     end
@@ -299,7 +311,7 @@ def sandbox_root
       File.join(Workspace.dir, 'sandbox')
     end
 
-    def reconfigure(options)
+    def reconfigure(options={})
       @user_authentication = options.fetch(:user_authentication, 'local')
       @config_server_enabled = options.fetch(:config_server_enabled, false)
       @drop_database = options.fetch(:drop_database, false)
@@ -324,6 +336,14 @@ def reconfigure(options)
     def check_if_nats_need_reset(allow_legacy_clients)
       @nats_needs_restart = @nats_allow_legacy_clients != allow_legacy_clients
       @nats_allow_legacy_clients = allow_legacy_clients
+
+      if @nats_allow_legacy_clients
+        @nats_url = "nats://#{@nats_user}:#{@nats_password}@127.0.0.1:#{nats_port}"
+      else
+        @nats_url = "nats://127.0.0.1:#{nats_port}"
+      end
+
+      @cpi.options['nats'] = @nats_url
     end
 
     def certificate_path
@@ -357,7 +377,7 @@ def nats_certificate_paths
 
     def director_nats_config
       {
-        uri: "nats://localhost:#{nats_port}",
+        uri: "nats://127.0.0.1:#{nats_port}",
         ssl: true,
         tls: {
           :private_key_file => nats_certificate_paths['clients']['test_client']['private_key_path'],
@@ -394,7 +414,7 @@ def external_cpi_config
         name: 'test-cpi',
         exec_path: File.join(REPO_ROOT, 'bosh-director', 'bin', 'dummy_cpi'),
         job_path: sandbox_path(EXTERNAL_CPI),
-        config_path: sandbox_path(DirectorService::DEFAULT_DIRECTOR_CONFIG),
+        config_path: sandbox_path(EXTERNAL_CPI_CONFIG),
         env_path: ENV['PATH'],
         gem_home: ENV['GEM_HOME'],
         gem_path: ENV['GEM_PATH']
@@ -425,6 +445,7 @@ def do_reset
         @nats_process.stop
         nats_template_path = File.join(SANDBOX_ASSETS_DIR, DEFAULT_NATS_CONF_TEMPLATE_NAME)
         write_in_sandbox(NATS_CONFIG, load_config_template(nats_template_path))
+        write_in_sandbox(EXTERNAL_CPI_CONFIG, load_config_template(EXTERNAL_CPI_CONFIG_TEMPLATE))
         setup_nats
         @nats_process.start
         @nats_socket_connector.try_to_connect
@@ -444,6 +465,7 @@ def setup_sandbox_root
       hm_template_path = File.join(SANDBOX_ASSETS_DIR, DEFAULT_HM_CONF_TEMPLATE_NAME)
       write_in_sandbox(HM_CONFIG, load_config_template(hm_template_path))
       write_in_sandbox(EXTERNAL_CPI, load_config_template(EXTERNAL_CPI_TEMPLATE))
+      write_in_sandbox(EXTERNAL_CPI_CONFIG, load_config_template(EXTERNAL_CPI_CONFIG_TEMPLATE))
       nats_template_path = File.join(SANDBOX_ASSETS_DIR, DEFAULT_NATS_CONF_TEMPLATE_NAME)
       write_in_sandbox(NATS_CONFIG, load_config_template(nats_template_path))
       FileUtils.chmod(0755, sandbox_path(EXTERNAL_CPI))

diff --git a/src/bosh-director/bin/dummy_cpi b/src/bosh-director/bin/dummy_cpi
@@ -47,18 +47,17 @@ result = nil
 error = nil
 
 begin
-  director_config = YAML.load_file(ARGV.shift)
-  cloud_properties = director_config['cloud']['properties']
+  cpi_config = JSON.parse(File.read(ARGV.shift))
   log_buffer = StringIO.new
-  cloud_properties['log_buffer'] = log_buffer
+  cpi_config['log_buffer'] = log_buffer
 
   request = JSON.parse($stdin.readline)
 
   command = request['method']
   arguments = request['arguments']
   context = request['context']
 
-  dummy = Bosh::Clouds::Dummy.new(cloud_properties, context)
+  dummy = Bosh::Clouds::Dummy.new(cpi_config, context)
 
   result = dummy.send(command, *arguments)
 rescue => e

diff --git a/src/bosh-director/lib/bosh/director/config.rb b/src/bosh-director/lib/bosh/director/config.rb
@@ -132,7 +132,7 @@ def configure(config)
         @nats_client_ca_certificate_path = config['nats']['client_ca_certificate_path']
         @nats_client_ca_private_key_path = config['nats']['client_ca_private_key_path']
         @nats_server_ca = File.read(@nats_server_ca_path)
-        
+
         @default_ssh_options = config['default_ssh_options']
 
         @cloud_options = config['cloud']

diff --git a/src/bosh-director/lib/cloud/dummy.rb b/src/bosh-director/lib/cloud/dummy.rb
@@ -11,10 +11,12 @@ class Dummy
       class NotImplemented < StandardError; end
 
       attr_reader :commands
+      attr_accessor :options
 
       def initialize(options, context)
         @options = options
 
+
         @base_dir = options['dir']
         if @base_dir.nil?
           raise ArgumentError, 'Must specify dir'

diff --git a/src/bosh-monitor/lib/bosh/monitor/runner.rb b/src/bosh-monitor/lib/bosh/monitor/runner.rb
@@ -74,8 +74,6 @@ def connect_to_mbus
 
       nats_client_options = {
         :uri       => @mbus.endpoint,
-        :user      => @mbus.user,
-        :pass      => @mbus.password,
         :autostart => false,
         :tls => {
           :ca_file => @mbus.server_ca_path,