make dynamic settings writable in dev/oss

closes AE-567
flag=none

test plan:
-specs pass

Change-Id: I1a0309d14c195053a04662e72ecb13a65efb74ce
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/337226
QA-Review: Jake Oeding <jake.oeding@instructure.com>
Product-Review: Jake Oeding <jake.oeding@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Build-Review: Jacob Burroughs <jburroughs@instructure.com>

config/initializers/active_record.rb

@@ -2213,7 +2213,7 @@ module AdditionalIgnoredColumns
       cache_class = ActiveRecord::Base.singleton_class
       return super unless cache_class.columns_to_ignore_enabled
 
-      cache_class.columns_to_ignore_cache[table_name] ||= DynamicSettings.find("activerecord/ignored_columns", tree: :store)[table_name, failsafe: ""]&.split(",") || []
+      cache_class.columns_to_ignore_cache[table_name] ||= DynamicSettings.find("activerecord/ignored_columns", tree: :store, ignore_fallback_overrides: true)[table_name, failsafe: ""]&.split(",") || []
       super + cache_class.columns_to_ignore_cache[table_name]
     end
   end
@@ -2223,7 +2223,7 @@ module AdditionalIgnoredColumns
 
     def reset_ignored_columns!
       @columns_to_ignore_cache = {}
-      @columns_to_ignore_enabled = !ActiveModel::Type::Boolean.new.cast(DynamicSettings.find("activerecord", tree: :store)["ignored_columns_disabled", failsafe: false])
+      @columns_to_ignore_enabled = !ActiveModel::Type::Boolean.new.cast(DynamicSettings.find("activerecord", tree: :store, ignore_fallback_overrides: true)["ignored_columns_disabled", failsafe: false])
     end
   end
 end

docker-compose/config/dynamic_settings.yml

@@ -34,6 +34,10 @@ development:
       ios-pandata-secret: teamrocketblastoffatthespeedoflight
       android-pandata-key: ANDROID_pandata_key
       android-pandata-secret: surrendernoworpreparetofight
+  # Key/value pairs in the store tree can have overrides
+  # persisted in Settings. They are included by default but
+  # can be ignored by passing 'ignore_fallback_overrides: true'
+  # to your DynamicSettings.find call.
   store:
     canvas:
       lti-keys:

gems/dynamic_settings/lib/dynamic_settings.rb

@@ -34,8 +34,8 @@ module DynamicSettings
   CACHE_KEY_PREFIX = "dynamic_settings/"
 
   class << self
-    attr_accessor :environment
-    attr_reader :fallback_data, :use_consul, :config
+    attr_accessor :environment, :fallback_data
+    attr_reader :use_consul, :config
     attr_writer :fallback_recovery_lambda, :retry_lambda, :cache, :request_cache, :logger
 
     def config=(conf_hash)
@@ -91,23 +91,18 @@ module DynamicSettings
     end
 
     def on_reload!
-      @root_fallback_proxy = nil
       reset_cache!
     end
 
-    # Set the fallback data to use in leiu of Consul
+    # The fallback data to use in leiu of Consul
     #
     # This isn't really meant for use in production, but as a convenience for
     # development where most won't want to run a consul agent/server.
-    def fallback_data=(value)
-      @fallback_data = value
-      @root_fallback_proxy = if @fallback_data
-                               FallbackProxy.new(@fallback_data.with_indifferent_access)
-                             end
-    end
+    def config_data
+      # fallback_data is only relevant when set via accessor in specs
+      return @fallback_data if @fallback_data
 
-    def root_fallback_proxy
-      @root_fallback_proxy ||= FallbackProxy.new(ConfigFile.load("dynamic_settings").dup)
+      @config_data ||= ConfigFile.load("dynamic_settings")
     end
 
     # Build an object used to interacting with consul for the given
@@ -130,7 +125,8 @@ module DynamicSettings
              service: nil,
              cluster: nil,
              default_ttl: PrefixProxy::DEFAULT_TTL,
-             data_center: nil)
+             data_center: nil,
+             ignore_fallback_overrides: false)
       service ||= @default_service || :canvas
       if use_consul
         PrefixProxy.new(
@@ -147,7 +143,7 @@ module DynamicSettings
           circuit_breaker: @config.fetch("circuit_breaker", nil)
         )
       else
-        proxy = root_fallback_proxy
+        proxy = FallbackProxy.new(config_data.deep_dup, ignore_fallback_overrides:)
         proxy = proxy.for_prefix(tree)
         proxy = proxy.for_prefix(service)
         proxy = proxy.for_prefix(prefix) if prefix

gems/dynamic_settings/lib/dynamic_settings/fallback_proxy.rb

@@ -15,12 +15,19 @@
 #
 # You should have received a copy of the GNU Affero General Public License along
 # with this program. If not, see <http://www.gnu.org/licenses/>.
+
 module DynamicSettings
   class FallbackProxy
+    PERSISTENCE_FLAG = "dynamic_settings:"
+    PERSISTED_TREE = "store"
+
     attr_reader :data
 
-    def initialize(data = nil)
+    def initialize(data = nil, path = nil, ignore_fallback_overrides: false)
       @data = (data || {}).with_indifferent_access
+      @path = path
+      @ignore_fallback_overrides = ignore_fallback_overrides
+      load_fallback_overrides if load_overrides?
     end
 
     def fetch(key, **_)
@@ -38,10 +45,47 @@ module DynamicSettings
     # @return [Hash]
     def set_keys(kvs, global: nil)
       @data.merge!(kvs)
+      if overridable?
+        kvs.each do |k, v|
+          Setting.set(PERSISTENCE_FLAG + append_path(k), v)
+        end
+      end
     end
 
     def for_prefix(prefix_extension, **_)
-      self.class.new(@data[prefix_extension])
+      self.class.new(
+        @data[prefix_extension],
+        append_path(prefix_extension),
+        ignore_fallback_overrides: @ignore_fallback_overrides
+      )
+    end
+
+    private
+
+    def load_overrides?
+      !@ignore_fallback_overrides && @path == PERSISTED_TREE
+    end
+
+    def overridable?
+      !@ignore_fallback_overrides && @path&.starts_with?(PERSISTED_TREE)
+    end
+
+    def load_fallback_overrides
+      overrides = Setting.where("name LIKE ?", "#{PERSISTENCE_FLAG + @path}%")
+      overrides.each do |setting|
+        _tree, *segments, key = setting.name.delete_prefix(PERSISTENCE_FLAG).split("/")
+        prefix = @data
+        segments.each do |part|
+          prefix[part] ||= {}
+          prefix = prefix[part]
+        end
+        prefix[key] = setting.value
+      end
+    end
+
+    def append_path(prefix)
+      prefix_string = prefix.to_s
+      @path.nil? ? prefix_string : @path + "/" + prefix_string
     end
   end
 end

gems/dynamic_settings/spec/dynamic_settings/fallback_proxy_spec.rb

@@ -16,6 +16,7 @@
 # You should have received a copy of the GNU Affero General Public License along
 # with this program. If not, see <http://www.gnu.org/licenses/>.
 require "spec_helper"
+require "fixtures/setting"
 
 module DynamicSettings
   RSpec.describe FallbackProxy do
@@ -29,12 +30,38 @@ module DynamicSettings
     end
 
     let(:proxy) { FallbackProxy.new(fallback_data) }
+    let(:foo_key) { FallbackProxy::PERSISTENCE_FLAG + FallbackProxy::PERSISTED_TREE + "/foo" }
+    let(:baz_key) { FallbackProxy::PERSISTENCE_FLAG + "baz/qux" }
+    let(:foo_setting) { Setting.new(foo_key, "override") }
+    let(:baz_setting) { Setting.new(baz_key, "override") }
 
     describe "#initalize" do
+      before { allow(Setting).to receive(:where).and_return([foo_setting]) }
+
       it "must store an empty hash when initialized with nil" do
         proxy = FallbackProxy.new(nil)
         expect(proxy.data).to eq({})
       end
+
+      describe "with ignore_fallback_overrides: true" do
+        it "does not load overrides from Settings" do
+          proxy = FallbackProxy.new(fallback_data, FallbackProxy::PERSISTED_TREE, ignore_fallback_overrides: true)
+          expect(proxy[:foo]).to eq "bar"
+        end
+      end
+
+      describe "with ignore_fallback_overrides: false" do
+        it "loads overrides from Settings for PERSISTED_TREE" do
+          proxy = FallbackProxy.new(fallback_data, FallbackProxy::PERSISTED_TREE, ignore_fallback_overrides: false)
+          expect(proxy[:foo]).to eq "override"
+        end
+
+        it "does not load overrides from Settings for other trees" do
+          allow(Setting).to receive(:where).and_return([baz_setting])
+          proxy = FallbackProxy.new(fallback_data, ignore_fallback_overrides: false).for_prefix("baz")
+          expect(proxy[:qux]).to eq "42"
+        end
+      end
     end
 
     describe "#fetch(key, ttl: nil)" do
@@ -60,6 +87,30 @@ module DynamicSettings
         proxy.set_keys(kvs)
         expect(proxy.data).to include kvs
       end
+
+      describe "with ignore_fallback_overrides: true" do
+        it "does not persist writes to PERSISTED_TREE in Settings" do
+          proxy = FallbackProxy.new(fallback_data, FallbackProxy::PERSISTED_TREE, ignore_fallback_overrides: true)
+          expect(Setting).not_to receive(:set)
+          proxy.set_keys({ foo: "1" })
+        end
+      end
+
+      describe "with ignore_fallback_overrides: false" do
+        before { allow(Setting).to receive(:where).and_return([foo_setting]) }
+
+        it "persists writes to PERSISTED_TREE in Settings" do
+          proxy = FallbackProxy.new(fallback_data, FallbackProxy::PERSISTED_TREE, ignore_fallback_overrides: false)
+          expect(Setting).to receive(:set).with(foo_key, "2")
+          proxy.set_keys({ foo: "2" })
+        end
+
+        it "does not persist writes to other trees in Settings" do
+          proxy = FallbackProxy.new(fallback_data, ignore_fallback_overrides: false)
+          expect(Setting).not_to receive(:set)
+          proxy.set_keys({ bzz: "3" })
+        end
+      end
     end
   end
 end

gems/dynamic_settings/spec/dynamic_settings_spec.rb

@@ -76,22 +76,6 @@ describe DynamicSettings do
     end
   end
 
-  describe ".fallback_data =" do
-    it "must provide indifferent access on resulting proxy" do
-      DynamicSettings.fallback_data = { foo: "bar" }
-      proxy = DynamicSettings.root_fallback_proxy
-      expect(proxy["foo"]).to eq "bar"
-      expect(proxy[:foo]).to eq "bar"
-    end
-
-    it "must clear the fallback when passed nil" do
-      DynamicSettings.fallback_data = { foo: "bar" }
-      DynamicSettings.fallback_data = nil
-      proxy = DynamicSettings.root_fallback_proxy
-      expect(proxy["foo"]).to be_nil
-    end
-  end
-
   describe ".find" do
     context "when consul is configured" do
       before do

gems/dynamic_settings/spec/fixtures/setting.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+# Copyright (C) 2024 - present Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+
+class Setting
+  class << self
+    def set(key, value) end
+
+    def where(*args) end
+  end
+
+  def initialize(name, value)
+    @name = name
+    @value = value
+  end
+  attr_reader :name, :value
+end

inst-cli/docker-compose/config/dynamic_settings.yml.erb

@@ -41,6 +41,10 @@ development:
       ios-pandata-secret: teamrocketblastoffatthespeedoflight
       android-pandata-key: ANDROID_pandata_key
       android-pandata-secret: surrendernoworpreparetofight
+  # Key/value pairs in the store tree can have overrides
+  # persisted in Settings. They are included by default but
+  # can be ignored by passing 'ignore_fallback_overrides: true'
+  # to your DynamicSettings.find call.
   store:
     canvas:
       lti-keys:

spec/controllers/eportfolios_controller_spec.rb

@@ -75,8 +75,8 @@ describe EportfoliosController do
 
       it "exposes the feature state for rich content service to js_env" do
         allow(DynamicSettings).to receive(:find).with("rich-content-service", default_ttl: 5.minutes).and_return(
-          DynamicSettings::FallbackProxy.new("app-host" => "rce.docker",
-                                             "cdn-host" => "rce.docker")
+          DynamicSettings::FallbackProxy.new({ "app-host" => "rce.docker",
+                                               "cdn-host" => "rce.docker" })
         )
         get "user_index"
         expect(response).to be_successful

spec/controllers/info_controller_spec.rb

@@ -173,7 +173,7 @@ describe InfoController do
       allow(DynamicSettings).to receive(:find).with(any_args).and_call_original
       allow(DynamicSettings).to receive(:find)
         .with("rich-content-service")
-        .and_return(DynamicSettings::FallbackProxy.new("app-host" => "rce.instructure.com"))
+        .and_return(DynamicSettings::FallbackProxy.new({ "app-host" => "rce.instructure.com" }))
       allow(CanvasHttp).to receive(:get).with(any_args).and_return(success_response)
       allow(IncomingMailProcessor::IncomingMessageProcessor).to receive_messages(run_periodically: true, healthy?: true)
     end

spec/controllers/security_controller_spec.rb

@@ -27,11 +27,11 @@ RSpec.describe SecurityController, type: :request do
   let(:future_key) { CanvasSecurity::KeyStorage.new_key }
 
   let(:fallback_proxy) do
-    DynamicSettings::FallbackProxy.new(
-      CanvasSecurity::KeyStorage::PAST => past_key,
-      CanvasSecurity::KeyStorage::PRESENT => present_key,
-      CanvasSecurity::KeyStorage::FUTURE => future_key
-    )
+    DynamicSettings::FallbackProxy.new({
+                                         CanvasSecurity::KeyStorage::PAST => past_key,
+                                         CanvasSecurity::KeyStorage::PRESENT => present_key,
+                                         CanvasSecurity::KeyStorage::FUTURE => future_key
+                                       })
   end
 
   around do |example|

spec/factories/account_factory.rb

@@ -26,9 +26,7 @@ module Factories
     # make sure this is loaded first
     allow(DynamicSettings).to receive(:find).with(any_args).and_call_original
     allow(DynamicSettings).to receive(:find).with("rich-content-service", default_ttl: 5.minutes).and_return(
-      DynamicSettings::FallbackProxy.new(
-        "app-host": ENV["RCE_HOST"] || "http://localhost:3001"
-      )
+      DynamicSettings::FallbackProxy.new({ "app-host": ENV["RCE_HOST"] || "http://localhost:3001" })
     )
 
     allow(Rails.application.credentials).to receive(:dig).and_call_original

spec/helpers/datadog_rum_helper_spec.rb

@@ -24,11 +24,11 @@ describe DatadogRumHelper do
   include ApplicationHelper
 
   let(:datadog_rum_config) do
-    DynamicSettings::FallbackProxy.new(
-      application_id: "27627d1e-8a4f-4645-b390-bb396fc83c81",
-      client_token: "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r",
-      sample_rate_percentage: 100.0
-    )
+    DynamicSettings::FallbackProxy.new({
+                                         application_id: "27627d1e-8a4f-4645-b390-bb396fc83c81",
+                                         client_token: "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r",
+                                         sample_rate_percentage: 100.0
+                                       })
   end
 
   describe "#include_datadog_rum_js?" do

spec/initializers/active_record_spec.rb

@@ -365,8 +365,8 @@ module ActiveRecord
       end
 
       after do
-        allow(DynamicSettings).to receive(:find).with("activerecord/ignored_columns", tree: :store).and_call_original
-        allow(DynamicSettings).to receive(:find).with("activerecord/ignored_columns_disabled", tree: :store).and_call_original
+        allow(DynamicSettings).to receive(:find).with("activerecord/ignored_columns", tree: :store, ignore_fallback_overrides: true).and_call_original
+        allow(DynamicSettings).to receive(:find).with("activerecord/ignored_columns_disabled", tree: :store, ignore_fallback_overrides: true).and_call_original
 
         reset_cache!
         User.create!(name: "user u2")
@@ -378,12 +378,12 @@ module ActiveRecord
       end
 
       def set_ignored_columns_state!(columns, enabled)
-        allow(DynamicSettings).to receive(:find).with("activerecord", tree: :store).and_return(
-          DynamicSettings::FallbackProxy.new({ "ignored_columns_disabled" => !enabled })
+        allow(DynamicSettings).to receive(:find).with("activerecord", tree: :store, ignore_fallback_overrides: true).and_return(
+          DynamicSettings::FallbackProxy.new({ "ignored_columns_disabled" => !enabled }, ignore_fallback_overrides: true)
         )
 
-        allow(DynamicSettings).to receive(:find).with("activerecord/ignored_columns", tree: :store).and_return(
-          DynamicSettings::FallbackProxy.new({ "users" => columns })
+        allow(DynamicSettings).to receive(:find).with("activerecord/ignored_columns", tree: :store, ignore_fallback_overrides: true).and_return(
+          DynamicSettings::FallbackProxy.new({ "users" => columns }, ignore_fallback_overrides: true)
         )
 
         reset_cache!

spec/lib/services/rich_content_spec.rb

@@ -25,10 +25,10 @@ module Services
       allow(DynamicSettings).to receive(:find).with(any_args).and_call_original
       allow(DynamicSettings).to receive(:find)
         .with("rich-content-service", default_ttl: 5.minutes)
-        .and_return(DynamicSettings::FallbackProxy.new(
-                      "app-host" => "rce-app",
-                      "cdn-host" => "rce-cdn"
-                    ))
+        .and_return(DynamicSettings::FallbackProxy.new({
+                                                         "app-host" => "rce-app",
+                                                         "cdn-host" => "rce-cdn"
+                                                       }))
       allow(Setting).to receive(:get)
     end