Enable autotailor to process multi-profile JSON Tailorings #2146
Conversation
evgenyz
force-pushed
the
add-multiprofile-tailoring
branch
from
2f88cc2
to
592d2df
Compare
utils/autotailor.8
Outdated
| \fB--json-tailoring JSON_TAILORING_FILE\fR | ||
| .RS | ||
| Import tailoring from a JSON file (https://github.com/ComplianceAsCode/schemas/tree/main/tailoring). This option makes BASE_PROFILE_ID positional argument optional. | ||
| However, data passed in the command line options takes precedence over JSON contents, including the BASE_PROFILE_ID argument. | ||
| When a JSON tailoring is accompanied with additional adjustments from the command-line options either a new profile would be added to the resulting XCCDF tailoring file or the profile with TAILORED_PROFILE_ID identifier, if present, will be updated accordingly. |
There was a problem hiding this comment.
It doesn't seem to work the way it's documented here. For example:
utils/autotailor --id-namespace "com.example.www" --json-tailoring tests/utils/custom.json \
--select R999 tests/utils/data_stream.xml.
Updating an existing profile with `--tailored-profile-id JSON_P1` seems to work.
should create a new profile but it doesn't create a new profile, the output is the same as if run without the --select R999 option.
There was a problem hiding this comment.
Well, okay, the thing is that if you actually want a new profile you absolutely must provide base profile id. I can't make it a required parameter if --json-tailoring is present since it is ambiguous what the user actually wants (and depends on the content of the JSON file). I don't want to ruin backward compatibility, so that's what I came up with. If you have any suggestions on how to work around this, I'm all ears.
There was a problem hiding this comment.
My suggestion is to rephrase the doc text and help text. It needs to be clear that if the --json-tailoring option is set and other options are set at the same time, then the user needs to add --tailored-profile-id.
There was a problem hiding this comment.
It's not about --tailored-profile-id (it could be auto-generated) but about BASE_PROFILE positional argument. Which we can't deduce.
There was a problem hiding this comment.
but --tailored-profile-id works for me as well
There was a problem hiding this comment.
I've reworded the help message a bit.
| self.value_changes = set() | ||
| self.rules_to_select = set() | ||
| self.rules_to_unselect = set() | ||
| self.groups_to_select = set() |
There was a problem hiding this comment.
Wouldn't the lists be better to ensure deterministic output? With this change, the generated profiles are ordered randomly. The order or elements shouldn't matter for the functionality but could be unpleasant for users running diffs or our tests. I suggest using lists or keep sets but sort the items at serializing.
There was a problem hiding this comment.
Sets are now converted to sorted lists before being rendered as XML.
utils/autotailor
Outdated
| @@ -339,7 +367,9 @@ def get_parser(): | |||
| parser.add_argument( | |||
| "-j", "--json-tailoring", metavar="JSON_TAILORING_FILENAME", default="", | |||
| help="JSON Tailoring (https://github.com/ComplianceAsCode/schemas/blob/main/tailoring/schema.json) " | |||
| "filename.") | |||
| "filename. When a JSON tailoring is accompanied with additional adjustments from the command-line" | |||
There was a problem hiding this comment.
I suggest improving the help text to clarify that the JSON is an input file that needs to be created and supplied by the user ie. it isn't an output file generated by autotailor.
There was a problem hiding this comment.
Text has been improved.
evgenyz
force-pushed
the
add-multiprofile-tailoring
branch
2 times, most recently
from
c2f1c5d
to
32af898
Compare
The script will now accept multiple profiles in JSON Tailorings and also will use command-line options to update existing profiles or will create a new profile in the XCCDF tailoring.
evgenyz
force-pushed
the
add-multiprofile-tailoring
branch
from
32af898
to
2700210
Compare
The script will now accept multiple profiles in JSON Tailorings and also will use command-line options to update existing profiles or will create a new profile in the XCCDF tailoring.
Also, I've added a synonym option
--tailoring-profile-idfor the existing rather confusing option--new-profile-id.I've tried to keep backward-compatibility both in logic and in the CLI API.