Skip to content

Conversation

@dombizita
Copy link
Contributor

@dombizita dombizita commented Apr 4, 2024

What changes were proposed in this pull request?

I added config option to be able to whitelist the algorithms defined in hdds.secret.key.algorithm, defaults to *.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10639

How was this patch tested?

This config is not used currently, CI on my fork (unrelated failure): https://github.com/dombizita/ozone/actions/runs/8540401619

…ric secret keys

Change-Id: I79653b5e228c9122e149b315ca73d8425752efea
Change-Id: Ie970e49ad57505d7ff10092827a76c04ab8e8b47
Copy link
Contributor

@Galsza Galsza left a comment

Choose a reason for hiding this comment

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

+1 Thank you for the changes @dombizita LGTM
The test failure seems unrelated

Copy link
Contributor

@fapifta fapifta left a comment

Choose a reason for hiding this comment

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

Hi @dombizita thank you for the changes proposed.

In #6470 I suggested to use a different naming and values for the base option.

In order to define default values to certain whitelist option, the suggestion is to use the crypto compliance mode string in these whitelist names, so that we can independently define different whitelists for different compliance modes.

In order to do so, we might not need these configs at all... and this affects #6473 and #6477 as well...

I am thinking about an approach where we use a CRYPTO_COMPLIANCE tag for properties that can have a whitelist, and the name of the whitelist property then can be derived from the property name...
So in this particular case, if we tag hdds.secrect.key.algorithm with a CRYPTO_COMPLIANCE tag, then in the code where we get a configuration property, based on the tag we can do a check for the compliance mode, and then check the related whitelist property...
In the case the whitelist property does not exists, we can assume that the values for that particular property is unlimited, and thats it.
This way, we can easily mark the existing relevant properties, add the non-existent properties to the ozone-default.xml with the CRYPTO_COMPLIANCE and other tags set as needed, and do a generic check.
What do you think? Does that sound feasible?

Also CC @Galsza as he is the owner of #6473

@adoroszlai
Copy link
Contributor

On a related note, do we need one PR per config key before it's even used anywhere?

@fapifta fapifta closed this Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants