[DOCS] Migrating from grains.get_or_set_hash to sdb.get_or_set_hash

[hugo-ricateau]

Description According to the 3005 release note, grains.get_or_set_hash has been removed in favour of sdb.get_or_set_hash (#61691).

I've updated every occurrences of salt['grains.get_or_set_hash'](...) to salt['sdb.get_or_set_hash'](...) in the state tree. When applying it via sudo salt-call --local state.apply (note that I'm using salt in masterless mode), in place of the generated hash I had before, I'm now obtaining a False value.

Even the example from the documentation is failing to generate and return a value:

sudo salt-call --local sdb.get_or_set_hash 'SECRET_KEY' 50
local:
    False

I've tried to setup another database but none of the existing drivers seems to implement the .get_or_set_hash method.

Another question: previously, I was sharing the grains file across the nodes of the cluster trough a glusterfs mount point in order to share data between masterless minions (master-minion model is not suitable in this specific situation); when using default sdb database (documentation is unclear if a default database even exists or if one must necessarily define one with a specific driver), where is the data-storage located?

[welcome[bot]]

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey. Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

• Community Wiki
• Salt’s Contributor Guide
• Join our Community Slack
• IRC on LiberaChat
• Salt Project YouTube channel
• Salt Project Twitch channel

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

[OrangeDog]

I already fixed that specific example in #62627.

Alternatively, put grains.get_or_set_hash back because it's not inherently insecure. Somebody just panicked because of the "trusting grains for key acceptance / pillar mapping" issue and deleted it.

The get-or-set logic is a bit clunky to do in jinja, but that is also an option.

[hugo-ricateau]

Thanks for your reply @OrangeDog.

I already fixed that specific example in #62627.

Okey, so let's analyse

% sudo salt-call --local sdb.get_or_set_hash sdb://mymemcached/SECRET_KEY 50
local:
    sdb://mymemcached/SECRET_KEY

As far as I'm concerned, there are two confusing aspects on this specific example:

• Why is this returning the name of the key instead of the generated hash? In templating one needs the generated hash, not the querying key.
• What is mymemcached, and where it is defined? As I understand it, I should setup a database in minion configuration file (@/etc/salt/minion); however, as stated in the initial report, no available driver seems to implement the .get_or_set_hash method.

Alternatively, put grains.get_or_set_hash back because it's not inherently insecure. Somebody just panicked because of the "trusting grains for key acceptance / pillar mapping" issue and deleted it.

In which version is this supposed to be available again? As of 3005.1 it still fails with

% sudo salt-call --local grains.get_or_set_hash 'SECRET_KEY' 50
'grains.get_or_set_hash' is not available.

The get-or-set logic is a bit clunky to do in jinja, but that is also an option.

Could you provide a ressource or an exemple of what you think of?

[OrangeDog]

Why is this returning the name of the key

That is still incorrect and needs further fixes.

What is mymemcached?

The name of an sbd profile. Most module documentation has this problem in that the actual documentation is in a guide somewhere else that it doesn't link to.

In which version is this supposed to be available again?

It isn't, that's the problem. In my opinion it should have never been removed.

Could you provide a ressource or an exemple of what you think of?

{% set ns = namespace() %}
{% set ns .value = salt["grains.get"]("SECRET_KEY", None) %}
{% if ns.value is None %}
  {% set ns .value = salt["random.get_str"]() %}
  {% do salt["grains.set"]("SECRET_KEY", ns.value) %}
{% endif %}

[hugo-ricateau]

Do I need to open specific bug issues or is this one sufficient?

[lkubb]

* [...] however, as stated in the initial report, no [available driver](https://docs.saltproject.io/en/latest/ref/sdb/all/index.html#sdb-modules) seems to implement the `.get_or_set_hash` method.

It is sufficient for the drivers to implement get and set, the get_or_set_hash logic is implemented in the utils module, which is used by the sdb execution module.

[hugo-ricateau]

• [...] however, as stated in the initial report, no available driver seems to implement the .get_or_set_hash method.

It is sufficient for the drivers to implement get and set, the get_or_set_hash logic is implemented in the utils module, which is used by the sdb execution module.

I agree with you @lkubb that, in theory, it should; however, in practice, it does not seem to be the case: I tried calling sdb.get_or_set_hash against a database defined with a yaml driver, and it resulted in an uncaught NotImplementedError on salt-call.

[lkubb]

[...] I tried calling sdb.get_or_set_hash against a database defined with a yaml driver, and it resulted in an uncaught NotImplementedError on salt-call.

That's because the YAML sdb module does not implement set_: https://github.com/saltstack/salt/blob/501e2324720f1336c27390ee2949d1448c59e024/salt/sdb/yaml.py#L63-L67

[hugo-ricateau]

That's because the YAML sdb module does not implement set_

I was expecting such a reason; however, my point here is that such an implementation defeats the purpose of a generic db interface: requiring manual edits in place of automatic generation is a non-sens for a database interface.

In summary, I think there are several issues reported in this thread:

• grains.get_or_set_hash should be restored, and a post-mortem should conclude with the necessity of adding safeguards to prevent such breaking changes to be so casually deployed in the future.
• All sdb drivers should fully implement the interface (or being tagged as not fully functional).
• The documentation concerning the sdb module should be clarified: it harshly misses a generic description of how the backend works, and especially of a link to the documentation describing how to define a sdb database; then, this later page also misses a link to the list of available drivers, along with some explanation of what has been pointed-out in this thread.

[barbaricyawps]

Can either @OrangeDog or @hugo-ricateau provide some feedback about what action needs to be taken here from a docs perspective? I'll leave the needs-triage label on so that I take a look at this again next week.

[barbaricyawps]

I think that this is bigger than a docs issue. I'm going to remove the Docs label so that it will get picked up by the developer's triage process. The docs work will get picked up as a natural by-product of the development work requested here.

[vincent-olivert-riera]

Any update on this?

[jbondhus]

Is there any update on this? This issue is still happening on salt 3007, it returns the name of the key instead of the value. I can't use grains.get_or_set_hash either because it's been removed.