Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Security: Simplify assessment relationships #338

Merged
merged 2 commits into from
May 17, 2023
Merged

Security: Simplify assessment relationships #338

merged 2 commits into from
May 17, 2023

Conversation

puerco
Copy link
Collaborator

@puerco puerco commented May 16, 2023

As discussed in #331 we are considering simplifying the security relationships to a single hasAssessmentFor.

This PR removes the following relationship types and updates the markdowns to use hasAssessmentFor while introducing it into the core vocabulary:

  • hasCvssV2AssessmentFor
  • hasCvssV3AssessmentFor
  • hasEpssAssessmentFor
  • hasExploitCatalogAssessmentFor
  • hasSsvcAssessmentFor

/cc @tsteenbe @rnjudge @jeff-schutt

closes #331

@puerco
Security Drop assessment type relationships
2d0012a 
This commit drops the relationship types named after each assessment
to favor a single hasAssessmentFor type.

Signed-off-by: Adolfo García Veytia (Puerco) <puerco@chainguard.dev>
@puerco
Drop deprecated security relationships from vocabulary
c90383e 
This commit drops the deprecated security relationships from the core vocabulary.

Signed-off-by: Adolfo García Veytia (Puerco) <puerco@chainguard.dev>
@puerco puerco mentioned this pull request May 16, 2023
Closed
@puerco
Copy link
Collaborator Author

puerco commented May 16, 2023

Also tagging @armintaenzertng as I know they have opinions about this :)

rnjudge
rnjudge approved these changes May 16, 2023
View reviewed changes
Copy link
Collaborator

@rnjudge rnjudge left a comment

Choose a reason for hiding this comment

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

Looks good. LGTM

kestewart
kestewart previously requested changes May 16, 2023
View reviewed changes
Copy link
Contributor

@kestewart kestewart left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Like taking it down to one relationship, but should there be a parameter on the relationship that puts in the Assessment types?

@kestewart
Copy link
Contributor

kestewart commented May 16, 2023 •
edited
Loading

Change looks very good overall. Thanks for pulling this together Adolfo. Do we not need an "AssessmentType" so we don't loose the types of info.

@kestewart kestewart added this to the 3.0-rc2 milestone May 16, 2023
@puerco
Copy link
Collaborator Author

puerco commented May 16, 2023

The relationship type is just the hasAssessmentFor but the relationships themselves are subclassed elements so they have all the information required, both the type and the full assessment data.

@tsteenbe tsteenbe merged commit cb39f85 into spdx:main May 17, 2023
@puerco puerco mentioned this pull request May 18, 2023
Closed
@puerco
Copy link
Collaborator Author

puerco commented May 18, 2023

Opened #346 to address Kate's comments above.

@rnjudge rnjudge mentioned this pull request Jul 12, 2023
Closed
@armintaenzertng armintaenzertng mentioned this pull request Jul 14, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rnjudge rnjudge rnjudge approved these changes

@tsteenbe tsteenbe tsteenbe approved these changes

@kestewart kestewart kestewart left review comments

@jeff-schutt jeff-schutt Awaiting requested review from jeff-schutt

Assignees
No one assigned
Labels
Profile:Security
Projects
None yet
Milestone
3.0-rc2
Development

Successfully merging this pull request may close these issues.

Reassessing Security Profile Relationships
4 participants
@puerco @kestewart @tsteenbe @rnjudge