Skip to content

Conversation

simar7
Copy link
Member

@simar7 simar7 commented Jun 21, 2025

Description

Use id and long_id for misconfig checks

Related PRs

Related Discussions

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@simar7 simar7 force-pushed the id-cleanup-2 branch 3 times, most recently from 7d331c8 to 375c1ea Compare June 21, 2025 06:14
@simar7 simar7 force-pushed the id-cleanup-2 branch 5 times, most recently from c155464 to e6be970 Compare June 24, 2025 03:06
@simar7 simar7 force-pushed the id-cleanup-2 branch 3 times, most recently from 6ca5385 to 53d170a Compare June 24, 2025 21:46
@simar7 simar7 marked this pull request as ready for review June 24, 2025 22:39
@nikpivkin
Copy link
Contributor

@simar7 @itaysk Should the id now have the prefix AVD?

@simar7
Copy link
Member Author

simar7 commented Jun 25, 2025

@simar7 @itaysk Should the id now have the prefix AVD?

Actually, good question. I thought about it and there's a way to not have this, we'll have to add aliases.

I couldn't find any good way to automate this addition of aliases. I also didn't try hard enough but if we do decide to change IDs, let's do it in a separate PR.

It would be something along the lines of AWS0001, GCP0001 etc.

@nikpivkin
Copy link
Contributor

In that case, should we first discuss and decide on the new format of id checks and make all the changes in trivy-checks and then update trivy-checks in trivy and at the same time update id to the new format so that the changes are consistent and avoid errors?

@simar7
Copy link
Member Author

simar7 commented Jun 27, 2025

In that case, should we first discuss and decide on the new format of id checks and make all the changes in trivy-checks and then update trivy-checks in trivy and at the same time update id to the new format so that the changes are consistent and avoid errors?

I've updated the IDs here aquasecurity/trivy-checks#441

I also removed this logic which as we discussed offline, seems unnecessary 80046a0

@DmitriyLewen
Copy link
Contributor

I feel that these changes are very radical and could affect a lot of people.

What if we split the changes into two parts:
First, use AVDID as ID and mark AVDID as deprecated (in fact, for a while these will be duplicate values),
and only after some time remove AVDID as deprecated.
This way, people who are using AVDID will be able to see that the field is deprecated (e.g., in our new notifications) and switch to ID.
wdyt?

@nikpivkin
Copy link
Contributor

nikpivkin commented Jun 30, 2025

I feel that these changes are very radical and could affect a lot of people.

Simar created a discussion with the news a month ago #8969 . This will only affect users who process reports manually, such as using the ID field of a check from JSON.

AVDID as ID and mark AVDID as deprecated

Even then, it would be a breaking change because the ID would change.

@DmitriyLewen
Copy link
Contributor

Usually we mark a field as deprecated and only after a few releases (e.g. we remove the AWS command after 1 year) we remove the field.
But in this case it is not possible (because we not only remove the field, we change the value from avdid to id).
That's why I care about users.

But I don't know all the use cases. If you are sure that it is not that critical for users - let's remove the field

@nikpivkin
Copy link
Contributor

@simar7 @itaysk Should we move this PR to the next release?

@simar7
Copy link
Member Author

simar7 commented Jun 30, 2025

@simar7 @itaysk Should we move this PR to the next release?

Yes, I've already updated the discussion to do so #8969

@itaysk
Copy link
Contributor

itaysk commented Jun 30, 2025

Should we move this PR to the next release?

Yes I was going to suggest the same

@simar7 simar7 force-pushed the id-cleanup-2 branch 2 times, most recently from e209f11 to e9f1729 Compare August 6, 2025 01:31
Signed-off-by: nikpivkin <[email protected]>
@simar7 simar7 marked this pull request as ready for review August 6, 2025 22:40
@simar7
Copy link
Member Author

simar7 commented Sep 8, 2025

In that case, should we first discuss and decide on the new format of id checks and make all the changes in trivy-checks and then update trivy-checks in trivy and at the same time update id to the new format so that the changes are consistent and avoid errors?

@nikpivkin should we also bump the major version (for the checks bundle) as part of this PR?

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.

4 participants