🐛 Show vanished resources as unscored #131
Conversation
policy/executor/internal/nodes.go
Outdated
| @@ -315,6 +318,16 @@ func (nodeData *ReportingQueryNodeData) score() *policy.Score { | |||
| break | |||
| } | |||
|
|
|||
| if cur.Data.Error != nil && strings.HasSuffix(cur.Data.Error.Error(), "not found") { | |||
There was a problem hiding this comment.
i feel like it would be possible for a lot of different errors to have a "not found" error that doesn't mean the asset went away
There was a problem hiding this comment.
perhaps we can define some common errors and check for those. If the asset is disconnected, the provider should return that error type
There was a problem hiding this comment.
I've changed the error message where it is created to be more precise: mondoohq/cnquery#361
I think, now that I have the point where the message is created, I could also create a specific error type out of it.
| @@ -347,7 +360,16 @@ func (nodeData *ReportingQueryNodeData) score() *policy.Score { | |||
| } | |||
|
|
|||
| if allFound { | |||
| if foundError { | |||
| if assetVanishedDuringScan { | |||
There was a problem hiding this comment.
In general, a suggestion:
I feel like we should be able to mark assets themselves as ephemeral (can tie either with asset type or provider type maybe?) and act accordingly.
I think a better way to fix this would be when starting the scan: if the asset does not exist then, we should be able to report accordingly upstream.
There was a problem hiding this comment.
we should be able to mark assets themselves as ephemeral
That sound like a much bigger topic
I think a better way to fix this would be when starting the scan: if the asset does not exist then, we should be able to report accordingly upstream.
I'm not sure I understand your point completely. Would changing mondoohq/cnquery#361 into a specific error type be what you had in mind?
There was a problem hiding this comment.
I totally agree it's a bigger topic, for sure not something that can be done quickly.
I was thinking more along the lines of having a specific error type that can be thrown if the resource does not exist rather than relying on the message inside the error
There was a problem hiding this comment.
I implemented a k8s specific error type in cnquery and check for it: mondoohq/cnquery#361
czunker
changed the title
|
Tests will only pass when mondoohq/cnquery#361 is merged and updated here. |
czunker
force-pushed
the
christian/fix_vanished_resources
branch
from
883b02c
to
5675d11
Compare
DO NOT MERGE! This is work in progress! Signed-off-by: Christian Zunker <[email protected]>
Signed-off-by: Christian Zunker <[email protected]>
Signed-off-by: Christian Zunker <[email protected]>
Signed-off-by: Christian Zunker <[email protected]>
Signed-off-by: Christian Zunker <[email protected]>
Signed-off-by: Christian Zunker <[email protected]>
czunker
force-pushed
the
christian/fix_vanished_resources
branch
from
5675d11
to
2964577
Compare
policy/executor/internal/nodes.go
Outdated
| @@ -316,8 +320,13 @@ func (nodeData *ReportingQueryNodeData) score() *policy.Score { | |||
| } | |||
|
|
|||
| if cur.Data.Error != nil { | |||
| allSkipped = false | |||
| foundError = true | |||
| var k8sNotFoundErr *k8s_common.K8sResourceNotFound | |||
There was a problem hiding this comment.
we should avoid making this code dependent on k8s specific logic. Can we create an error of type asset disconnected or something like that
There was a problem hiding this comment.
What about this? https://github.com/mondoohq/cnquery/pull/380/files
There was a problem hiding this comment.
Updated the PR to include the new cnquery. Now I check for the general error.
Signed-off-by: Christian Zunker <[email protected]>
Signed-off-by: Christian Zunker <[email protected]>
We now check for a specific error type, to check whether a k8s resource vanished.
Signed-off-by: Christian Zunker [email protected]