Refactor grafana_datasource

Property defaults are removed from the type and where values are needed
on datasource creation, the defaults are in the provider.

If updating an existing datasource, only properties that you want to
manage have to be specified. The API needs the post to contain several
more fields, but these can be pulled from the existing state.

`uid` is added as an optional property and, (in versions that support
it), is used when updating a datasource, (instead of updating by `id`
which has been deprecated.)

Fetching and deleting datasources by `id` has also been deprecated in
Grafana 9 so is replaced by fetching and deleting by `name`.

The managing of multiple datasources is now quicker as the previous
implementation made a number of API calls that grew exponentially with
the number of datasources.

Fixes #229

For users not using `basic_auth` or `password` properties in their
datasources restores idempotency when using Grafana 9 (see #289)

(From Grafana 9 onwards, users must use `secure_json_data` but this is
not idempotent and making it behave 100% correctly is currently
impossible as the Grafana API purposefully never exposes this data.)

There are still a number of issues open around using datasources in more
than one organization.  None of these have been addressed in this
commit.

lib/puppet/provider/grafana_datasource/grafana.rb

@@ -50,211 +50,200 @@ def fetch_organization
     @fetch_organization
   end
 
-  def datasources
-    response = send_request('GET', format('%s/datasources', resource[:grafana_api_path]))
-    raise format('Fail to retrieve datasources (HTTP response: %s/%s)', response.code, response.body) if response.code != '200'
+  def datasource_by_name
+    response = send_request('GET', format('%s/datasources/name/%s', resource[:grafana_api_path], resource[:name]))
+    return nil if response.code == '404'
 
-    begin
-      datasources = JSON.parse(response.body)
-
-      datasources.map { |x| x['id'] }.map do |id|
-        response = send_request 'GET', format('%s/datasources/%s', resource[:grafana_api_path], id)
-        raise format('Failed to retrieve datasource %d (HTTP response: %s/%s)', id, response.code, response.body) if response.code != '200'
-
-        datasource = JSON.parse(response.body)
+    raise Puppet::Error, format('Failed to retrieve datasource %s (HTTP response: %s/%s)', resource[:name], response.code, response.body) if response.code != '200'
 
-        {
-          id: datasource['id'],
-          name: datasource['name'],
-          url: datasource['url'],
-          type: datasource['type'],
-          user: datasource['user'],
-          password: datasource['password'],
-          database: datasource['database'],
-          access_mode: datasource['access'],
-          is_default: datasource['isDefault'] ? :true : :false,
-          with_credentials: datasource['withCredentials'] ? :true : :false,
-          basic_auth: datasource['basicAuth'] ? :true : :false,
-          basic_auth_user: datasource['basicAuthUser'],
-          basic_auth_password: datasource['basicAuthPassword'],
-          json_data: datasource['jsonData'],
-          secure_json_data: datasource['secureJsonData']
-        }
+    begin
+      JSON.parse(response.body).transform_values do |v|
+        case v
+        when true
+          :true
+        when false
+          :false
+        else
+          v
+        end
       end
     rescue JSON::ParserError
       raise format('Failed to parse response: %s', response.body)
     end
   end
 
   def datasource
-    @datasource ||= datasources.find { |x| x[:name] == resource[:name] }
+    @datasource ||= datasource_by_name
     @datasource
   end
 
   attr_writer :datasource
 
-  def type
-    datasource[:type]
-  end
+  # Create setters for all properties just so they exist
+  mk_resource_methods # Creates setters for all properties
 
-  def type=(value)
-    resource[:type] = value
-    save_datasource
+  # Then override all of the getters
+  def type
+    datasource['type']
   end
 
   def url
-    datasource[:url]
-  end
-
-  def url=(value)
-    resource[:url] = value
-    save_datasource
+    datasource['url']
   end
 
   def access_mode
-    datasource[:access_mode]
-  end
-
-  def access_mode=(value)
-    resource[:access_mode] = value
-    save_datasource
+    datasource['access']
   end
 
   def database
-    datasource[:database]
-  end
-
-  def database=(value)
-    resource[:database] = value
-    save_datasource
+    datasource['database']
   end
 
   def user
-    datasource[:user]
-  end
-
-  def user=(value)
-    resource[:user] = value
-    save_datasource
+    datasource['user']
   end
 
   def password
-    datasource[:password]
-  end
-
-  def password=(value)
-    resource[:password] = value
-    save_datasource
+    datasource['password']
   end
 
   # rubocop:disable Naming/PredicateName
   def is_default
-    datasource[:is_default]
+    datasource['isDefault']
   end
 
-  def is_default=(value)
-    resource[:is_default] = value
-    save_datasource
-  end
   # rubocop:enable Naming/PredicateName
 
   def basic_auth
-    datasource[:basic_auth]
-  end
-
-  def basic_auth=(value)
-    resource[:basic_auth] = value
-    save_datasource
+    datasource['basicAuth']
   end
 
   def basic_auth_user
-    datasource[:basic_auth_user]
-  end
-
-  def basic_auth_user=(value)
-    resource[:basic_auth_user] = value
-    save_datasource
+    datasource['basicAuthUser']
   end
 
   def basic_auth_password
-    datasource[:basic_auth_password]
-  end
-
-  def basic_auth_password=(value)
-    resource[:basic_auth_password] = value
-    save_datasource
+    datasource['basicAuthPassword']
   end
 
   def with_credentials
-    datasource[:with_credentials]
+    datasource['withCredentials']
   end
 
-  def with_credentials=(value)
-    resource[:with_credentials] = value
-    save_datasource
+  def json_data
+    datasource['jsonData']
   end
 
-  def json_data
-    datasource[:json_data]
+  def id
+    datasource['id']
   end
 
-  def json_data=(value)
-    resource[:json_data] = value
-    save_datasource
+  def uid
+    datasource['uid']
   end
 
   def secure_json_data
-    datasource[:secure_json_data]
+    # The API never returns `secure` data, so we won't ever be able to tell if the current state is correct.
+    # TODO: Figure this out!!
+    {}
   end
 
-  def secure_json_data=(value)
-    resource[:secure_json_data] = value
-    save_datasource
-  end
+  def flush
+    return if resource['ensure'] == :absent
 
-  def save_datasource
     # change organizations
     response = send_request 'POST', format('%s/user/using/%s', resource[:grafana_api_path], fetch_organization[:id])
     raise format('Failed to switch to org %s (HTTP response: %s/%s)', fetch_organization[:id], response.code, response.body) unless response.code == '200'
 
+    # Build the `data` to POST/PUT by first creating a hash with some defaults which will be used if we're _creating_ a datasource
     data = {
-      name: resource[:name],
-      type: resource[:type],
-      url: resource[:url],
-      access: resource[:access_mode],
-      database: resource[:database],
-      user: resource[:user],
-      password: resource[:password],
-      isDefault: (resource[:is_default] == :true),
-      basicAuth: (resource[:basic_auth] == :true),
-      basicAuthUser: resource[:basic_auth_user],
-      basicAuthPassword: resource[:basic_auth_password],
-      withCredentials: (resource[:with_credentials] == :true),
-      jsonData: resource[:json_data],
-      secureJsonData: resource[:secure_json_data]
+      access: :direct,
+      isDefault: false,
+      basicAuth: false,
+      withCredentials: false,
     }
 
+    # If we're updating a datasource, merge in the current state (overwriting the defaults above)
+    unless datasource.nil?
+      data.merge!(datasource.transform_keys(&:to_sym).slice(
+                    :access,
+                    :basicAuth,
+                    :basicAuthUser,
+                    :basicAuthPassword,
+                    :database,
+                    :isDefault,
+                    :jsonData,
+                    :type,
+                    :url,
+                    :user,
+                    :password,
+                    :withCredentials,
+                    :uid
+                  ))
+    end
+
+    # Finally, merge in the properies the user has specified
+    data.merge!(
+      {
+        name: resource['name'],
+        access: resource['access_mode'],
+        basicAuth: resource['basic_auth'],
+        basicAuthUser: resource['basic_auth_user'],
+        basicAuthPassword: resource['basic_auth_password'],
+        database: resource['database'],
+        isDefault: resource['is_default'],
+        jsonData: resource['json_data'],
+        type: resource['type'],
+        url: resource['url'],
+        user: resource['user'],
+        password: resource['password'],
+        withCredentials: resource['with_credentials'],
+        secureJsonData: resource['secure_json_data'],
+        uid: resource['uid']
+      }.compact
+    )
+
+    # Puppet properties need to work with symbols, but the Grafana API will want to receive actual Booleans
+    data.transform_values! do |v|
+      case v
+      when :true
+        true
+      when :false
+        false
+      else
+        v
+      end
+    end
+
     if datasource.nil?
+      Puppet.debug 'Creating datasource'
       response = send_request('POST', format('%s/datasources', resource[:grafana_api_path]), data)
+    elsif uid.nil?
+      # This API call is deprecated in Grafana 9 so we only use it if our datasource doesn't have a uid (eg Grafana 6)
+      Puppet.debug 'Updating datasource by id'
+      response = send_request 'PUT', format('%s/datasources/%s', resource[:grafana_api_path], id), data
     else
-      data[:id] = datasource[:id]
-      response = send_request 'PUT', format('%s/datasources/%s', resource[:grafana_api_path], datasource[:id]), data
+      Puppet.debug 'Updating datasource by uid'
+      response = send_request 'PUT', format('%s/datasources/uid/%s', resource[:grafana_api_path], uid), data
     end
-    raise format('Failed to create save %s (HTTP response: %s/%s)', resource[:name], response.code, response.body) if response.code != '200'
+
+    raise format('Failed to create/update %s (HTTP response: %s/%s)', resource[:name], response.code, response.body) if response.code != '200'
 
     self.datasource = nil
   end
 
   def delete_datasource
-    response = send_request 'DELETE', format('%s/datasources/%s', resource[:grafana_api_path], datasource[:id])
+    response = send_request 'DELETE', format('%s/datasources/name/%s', resource[:grafana_api_path], resource[:name])
 
     raise format('Failed to delete datasource %s (HTTP response: %s/%s', resource[:name], response.code, response.body) if response.code != '200'
 
     self.datasource = nil
   end
 
   def create
-    save_datasource
+    # There's no sensible default for `type` when creating a new datasource so perform some validation here
+    # The actual creation happens when `flush` gets called.
+    raise Puppet::Error, 'type is required when creating a new datasource' if resource[:type].nil?
   end
 
   def destroy

lib/puppet/type/grafana_datasource.rb

@@ -37,6 +37,10 @@
     desc 'The password for the Grafana server'
   end
 
+  newproperty(:uid) do
+    desc 'An optional unique identifier for the datasource. Supported by grafana 7.3 onwards. If you do not specify this parameter, grafana will assign a uid for you'
+  end
+
   newproperty(:url) do
     desc 'The URL/Endpoint of the datasource'
   end
@@ -66,35 +70,29 @@
   newproperty(:access_mode) do
     desc 'Whether the datasource is accessed directly or not by the clients'
     newvalues(:direct, :proxy)
-    defaultto :direct
   end
 
   newproperty(:is_default) do
     desc 'Whether the datasource is the default one'
     newvalues(:true, :false)
-    defaultto :false
   end
 
   newproperty(:basic_auth) do
     desc 'Whether basic auth is enabled or not'
     newvalues(:true, :false)
-    defaultto :false
   end
 
   newproperty(:basic_auth_user) do
     desc 'The username for basic auth if enabled'
-    defaultto ''
   end
 
   newproperty(:basic_auth_password) do
     desc 'The password for basic auth if enabled'
-    defaultto ''
   end
 
   newproperty(:with_credentials) do
     desc 'Whether credentials such as cookies or auth headers should be sent with cross-site requests'
     newvalues(:true, :false)
-    defaultto :false
   end
 
   newproperty(:json_data) do