Skip to content

Conversation

@evgenyz
Copy link
Contributor

@evgenyz evgenyz commented Jun 25, 2020

This introduces YAML Path selection $.blah['key1','key2'] syntax and
enriches yamlfilecontent test and schema with EntityStateRecordType and
EntityItemRecordType result elements for collecting and checking
complex objects in YAML/JSON documents.

This change also depends on removal of the 'only lower case' restriction
from the name attribute of the field element (EntityXxxFieldType).

@evgenyz
Copy link
Contributor Author

evgenyz commented Jun 25, 2020

Example of a check for the "Upgradable" problem:

    <ind-def:yamlfilecontent_object version="1" id="oval:0:obj:5">
      <ind-def:path>/tmp</ind-def:path>
      <ind-def:filename>openshift-logging.yaml</ind-def:filename>
      <ind-def:yamlpath>.status.conditions[:]['status','type']</ind-def:yamlpath>
    </ind-def:yamlfilecontent_object>

    <ind-def:yamlfilecontent_state version="1" id="oval:0:ste:5">
      <ind-def:value datatype="record">
        <field name="status" datatype="string">True</field>
        <field name="type" datatype="string">Upgradeable</field>
      </ind-def:value>
    </ind-def:yamlfilecontent_state>

More details in tests/probes/yamlfilecontent/test_probes_yamlfilecontent_key.xml.

CC @JAORMX

@evgenyz
Copy link
Contributor Author

evgenyz commented Jun 25, 2020

Problems so far: this thing

<field name="value_of" datatype="string" var_ref="oval:0:var:2" var_check="at least one" entity_check="at least one"/>

is not working, no idea why. (CC @jan-cerny)

Edit: fixed.

@evgenyz
Copy link
Contributor Author

evgenyz commented Jun 25, 2020

As a bonus side of this change – we can now address any map, given that there is no values with complex objects (selection syntax could help if that's not the case).

@evgenyz evgenyz force-pushed the improve-yaml-probe branch from 5941773 to a648dc9 Compare June 25, 2020 22:15
@evgenyz evgenyz requested review from jan-cerny and yuumasato June 25, 2020 22:17
@evgenyz evgenyz added this to the 1.3.4 milestone Jun 25, 2020
@evgenyz evgenyz force-pushed the improve-yaml-probe branch from a648dc9 to c4f3537 Compare June 26, 2020 05:42
@evgenyz
Copy link
Contributor Author

evgenyz commented Jun 26, 2020

This is a possible fix for ComplianceAsCode/content#5822.

@evgenyz evgenyz changed the title probes/yamlfilecontent: Bump yaml-filter, extend the schema and probe to be able to work with a set of values in mapsbe able to work with a set of values in maps probes/yamlfilecontent: Bump yaml-filter, extend the schema and probe to be able to work with a set of values in maps Jun 26, 2020
@yuumasato
Copy link
Member

Problems so far: this thing

<field name="value_of" datatype="string" var_ref="oval:0:var:2" var_check="at least one" entity_check="at least one"/>

is not working, no idea why. (CC @jan-cerny)

It could be because record data type does not allow variables.
https://github.com/OVALProject/Language/blob/master/docs/oval-common-schema.md#---complexdatatypeenumeration---

@evgenyz
Copy link
Contributor Author

evgenyz commented Jun 30, 2020

Problems so far: this thing

<field name="value_of" datatype="string" var_ref="oval:0:var:2" var_check="at least one" entity_check="at least one"/>

is not working, no idea why. (CC @jan-cerny)

It could be because record data type does not allow variables.
https://github.com/OVALProject/Language/blob/master/docs/oval-common-schema.md#---complexdatatypeenumeration---

Yeah, but it's a field (child of it, not record itself). I'm not sure here, but I understood that should be possible for field.

Copy link
Contributor

@matejak matejak left a comment

Choose a reason for hiding this comment

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

I have provided some comments about the OVAL.

Would it be possible to split the C code into more functions that would make it more obvious what is going on there? The code is starting to be complex, and it is said that function names document better than comments that tend to become obsolete very quickly.

Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

The idea to use record datatype is interesting, and makes sense to me.
There are a few caveats that should be considered though:

  • No variables
  • Only equals operator, no pattern matching
  • name should be all lower case

Edit: the remarks on the standard are about the record not the field.

@yuumasato
Copy link
Member

Yeah, but it's a field (child of it, not record itself). I'm not sure here, but I understood that should be possible for field.

Well noted, it makes sense that records cannot be compared to variables, and there is nothing related to fields comparison with variables.

@evgenyz
Copy link
Contributor Author

evgenyz commented Jun 30, 2020

Would it be possible to split the C code into more functions that would make it more obvious what is going on there? The code is starting to be complex, and it is said that function names document better than comments that tend to become obsolete very quickly.

I will brush it up during the final implementation of all the details we are currently discussing (like having value_of entry together with result and so on). This version was made with the least amount of code changes in mind to show what is actually going on.

@evgenyz
Copy link
Contributor Author

evgenyz commented Jul 15, 2020

Problems so far: this thing

<field name="value_of" datatype="string" var_ref="oval:0:var:2" var_check="at least one" entity_check="at least one"/>

is not working, no idea why. (CC @jan-cerny)

It could be because record data type does not allow variables.
https://github.com/OVALProject/Language/blob/master/docs/oval-common-schema.md#---complexdatatypeenumeration---

Yeah, but it's a field (child of it, not record itself). I'm not sure here, but I understood that should be possible for field.

So, it just wasn't implemented. I fixed it.

@evgenyz evgenyz force-pushed the improve-yaml-probe branch from 396cfd4 to bc37fce Compare July 17, 2020 07:18
Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

I'd go with value.
Changes in the scanner look good to me.

@evgenyz evgenyz force-pushed the improve-yaml-probe branch 2 times, most recently from 37f4fb6 to be38435 Compare July 23, 2020 05:32
@evgenyz evgenyz marked this pull request as ready for review July 23, 2020 05:32
@evgenyz
Copy link
Contributor Author

evgenyz commented Jul 31, 2020

All current issues were addressed, waiting for reaction in OVAL-Community/OVAL#91.

@evgenyz evgenyz force-pushed the improve-yaml-probe branch 2 times, most recently from 571d179 to 46cf732 Compare September 28, 2020 21:00
Add proper evaluation implementation for operation, var_check
and var_ref attributes in the field entry of a record.
be able to work with a set of values in maps

This introduces YAML Path selection ($.blah['key1','key2']) syntax and
enriches yamlfilecontent test and schema with EntityStateRecordType and
EntityItemRecordType result elements for collecting and checking
complex objects in YAML/JSON documents.

This change also depend on removal of the 'only lower case' restriction
from the name attribute of the field element (EntityXxxFieldType).
And adjust probe implementation and tests for the workaround (lowercase
and escape capital letters in field names with ^, use # instead
of empty field names for scalars).
@jan-cerny
Copy link
Member

looks great

@jan-cerny jan-cerny self-assigned this Oct 1, 2020
@jan-cerny
Copy link
Member

Let's merge it now, any other improvements can be opened separately.

@jan-cerny jan-cerny merged commit 0693cb3 into OpenSCAP:maint-1.3 Oct 1, 2020
@evgenyz evgenyz deleted the improve-yaml-probe branch July 9, 2021 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants