Skip to content

fix: Condition "Unknown" was shown for the condition without containing the status body #408

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

Merged
kyasbal merged 3 commits into from
Dec 9, 2025
Merged

fix: Condition "Unknown" was shown for the condition without containing the status body #408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e2eda41
fix: Condition "Unknown" was shown for the condition without containi…
kyasbal Dec 9, 2025
6951641
Update pkg/task/inspection/commonlogk8sauditv2/impl/condition_history…
kyasbal Dec 9, 2025
360957f
Update pkg/task/inspection/commonlogk8sauditv2/impl/condition_history…
kyasbal Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/model/history/timeline_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (b *TimelineBuilder) AddRevision(revision *ResourceRevision) {
b.timeline.Revisions = append(b.timeline.Revisions, revision)
if len(timeline.Revisions) >= 2 {
prev := timeline.Revisions[len(timeline.Revisions)-2]
if b.timeDiffOfLogIndicces(revision.Log, prev.Log) < 0 {
if revision.ChangeTime.Sub(prev.ChangeTime) < 0 {

@jyane jyane Dec 9, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this change related to the main concern of this PR?

@kyasbal kyasbal Dec 9, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I could say that is another bug but this is affecting same issue. The implicit "Unknown" condition generated in the previous version will be overwritten by later resource update and there was a bug in this case.

For example,

# manifest reconstructed from PATCH request 1 @ 2025-01-02T00:00:00Z
status:
  conditions:
    - type: Ready
      lastHeartbeatTime: "2025-01-02T00:00:00Z"
# manifest reconstructed from UPDATE request 2 @ 2025-01-03T00:00:00Z
metadata:
  creationTimestamp: "2025-01-01T00:00:00Z"
...(omit)
status:
  conditions:
    - type: Read
      status: "True"
      lastHeartbeatTime: "2025-01-03T00:00:00Z"

Then these creates following conditions revisions:

  • At 2025-01-01T00:00:00Z: No enough information to determine the condition but we can infer the resource existence from the creationTimestamp field found in the 2nd request. (rev-1)
  • At 2025-01-02T00:00:00Z: The condition information is not available because the PATCH request doesn't contain the status field under the condition. (rev-2)
  • At 2025-01-03T00:00:00Z: The condition is changed to "True". (rev-3)

Revisions were sorted with associated log time stamp, it hides the new status written at rev-2 because these were wrongly sorted as rev2, rev1, rev3.
And the 1st revision and 2nd rectangle region becomes [2025-01-02T00:00:00Z, 2025-01-01T00:00:00Z)(rev-2). and [2025-01-01T00:00:00Z,2025-01-03T00:00:00Z)(rev-1). (Note the revision end time is the change time of the next revision element and this hides rev-2 by rendering rev-1 on it )
This bug was not a severe problem for the most of cases because revisions are written in chronological order. But this bug hides the new explicit "Unknown" status.

@jyane jyane Dec 9, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for your explanation, I think we need this fix as well.

b.sorted = false
}
}
Expand Down
10 changes: 7 additions & 3 deletions pkg/task/inspection/commonlogk8sauditv2/impl/condition_historymodifier_task.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,16 @@ var _ commonlogk8sauditv2_contract.ManifestHistoryModifierTaskSetting[*condition

// conditionStateToRevisionState converts a Kubernetes condition status string ("True", "False", etc.) to a KHI RevisionState enum.
func conditionStateToRevisionState(conditionState string) enum.RevisionState {
if conditionState == "True" {
switch conditionState {
case "True":
return enum.RevisionStateConditionTrue
} else if conditionState == "False" {
case "False":
return enum.RevisionStateConditionFalse
case "":
return enum.RevisionStateConditionNoAvailableInfo
default:
return enum.RevisionStateConditionUnknown
}
return enum.RevisionStateConditionUnknown
}

type conditionWalker struct {
Expand Down
275 changes: 171 additions & 104 deletions pkg/task/inspection/commonlogk8sauditv2/impl/condition_historymodifier_task_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ func TestConditionWalker(t *testing.T) {
ParentRelationship: enum.RelationshipChild,
}
conditionType := "Ready"
walker := newConditionWalker(parentPath, conditionType)

baseTime := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
commonFieldSet := &log.CommonFieldSet{
Expand All @@ -49,122 +48,190 @@ func TestConditionWalker(t *testing.T) {
Principal: "user-1",
}

tests := []struct {
type step struct {
name string
condition *model.K8sResourceStatusCondition
want *history.StagingResourceRevision
}

scenarios := []struct {
name string
steps []step
}{
{
name: "Initial Condition (TransitionTime)",
condition: &model.K8sResourceStatusCondition{
Type: conditionType,
Status: "True",
LastTransitionTime: baseTime.Format(time.RFC3339),
},
want: &history.StagingResourceRevision{
Verb: enum.RevisionVerbUpdate,
Body: "lastTransitionTime: \"2024-01-01T00:00:00Z\"\nstatus: \"True\"\ntype: Ready\n",
Partial: false,
Requestor: "user-1",
ChangeTime: baseTime,
State: enum.RevisionStateConditionTrue,
},
},
{
name: "No Change",
condition: &model.K8sResourceStatusCondition{
Type: conditionType,
Status: "True",
LastTransitionTime: baseTime.Format(time.RFC3339),
},
want: nil,
},
{
name: "Status Change (TransitionTime)",
condition: &model.K8sResourceStatusCondition{
Type: conditionType,
Status: "False",
LastTransitionTime: baseTime.Add(1 * time.Hour).Format(time.RFC3339),
},
want: &history.StagingResourceRevision{
Verb: enum.RevisionVerbUpdate,
Body: "lastTransitionTime: \"2024-01-01T01:00:00Z\"\nstatus: \"False\"\ntype: Ready\n",
Partial: false,
Requestor: "user-1",
ChangeTime: baseTime.Add(1 * time.Hour),
State: enum.RevisionStateConditionFalse,
},
},
{
name: "Probe Time Change (ProbeLikeTime)",
condition: &model.K8sResourceStatusCondition{
Type: conditionType,
Status: "False",
LastTransitionTime: baseTime.Add(1 * time.Hour).Format(time.RFC3339),
LastHeartbeatTime: baseTime.Add(2 * time.Hour).Format(time.RFC3339),
},
want: &history.StagingResourceRevision{
Verb: enum.RevisionVerbUpdate,
Body: "lastHeartbeatTime: \"2024-01-01T02:00:00Z\"\nlastTransitionTime: \"2024-01-01T01:00:00Z\"\nstatus: \"False\"\ntype: Ready\n",
Partial: false,
Requestor: "user-1",
ChangeTime: baseTime.Add(2 * time.Hour),
State: enum.RevisionStateConditionFalse,
},
},
{
name: "No change on LastTransitionTime but changes on LastHeartbeatTime",
condition: &model.K8sResourceStatusCondition{
Type: conditionType,
Status: "False",
LastTransitionTime: baseTime.Add(1 * time.Hour).Format(time.RFC3339),
LastHeartbeatTime: baseTime.Add(3 * time.Hour).Format(time.RFC3339),
},
want: &history.StagingResourceRevision{
Verb: enum.RevisionVerbUpdate,
Body: "lastHeartbeatTime: \"2024-01-01T03:00:00Z\"\nlastTransitionTime: \"2024-01-01T01:00:00Z\"\nstatus: \"False\"\ntype: Ready\n",
Partial: false,
Requestor: "user-1",
ChangeTime: baseTime.Add(3 * time.Hour),
State: enum.RevisionStateConditionFalse,
name: "Standard Lifecycle",
steps: []step{
{
name: "Initial Condition (TransitionTime)",
condition: &model.K8sResourceStatusCondition{
Type: conditionType,
Status: "True",
LastTransitionTime: baseTime.Format(time.RFC3339),
},
want: &history.StagingResourceRevision{
Verb: enum.RevisionVerbUpdate,
Body: "lastTransitionTime: \"2024-01-01T00:00:00Z\"\nstatus: \"True\"\ntype: Ready\n",
Partial: false,
Requestor: "user-1",
ChangeTime: baseTime,
State: enum.RevisionStateConditionTrue,
},
},
{
name: "No Change",
condition: &model.K8sResourceStatusCondition{
Type: conditionType,
Status: "True",
LastTransitionTime: baseTime.Format(time.RFC3339),
},
want: nil,
},
{
name: "Status Change (TransitionTime)",
condition: &model.K8sResourceStatusCondition{
Type: conditionType,
Status: "False",
LastTransitionTime: baseTime.Add(1 * time.Hour).Format(time.RFC3339),
},
want: &history.StagingResourceRevision{
Verb: enum.RevisionVerbUpdate,
Body: "lastTransitionTime: \"2024-01-01T01:00:00Z\"\nstatus: \"False\"\ntype: Ready\n",
Partial: false,
Requestor: "user-1",
ChangeTime: baseTime.Add(1 * time.Hour),
State: enum.RevisionStateConditionFalse,
},
},
{
name: "Probe Time Change (ProbeLikeTime)",
condition: &model.K8sResourceStatusCondition{
Type: conditionType,
Status: "False",
LastTransitionTime: baseTime.Add(1 * time.Hour).Format(time.RFC3339),
LastHeartbeatTime: baseTime.Add(2 * time.Hour).Format(time.RFC3339),
},
want: &history.StagingResourceRevision{
Verb: enum.RevisionVerbUpdate,
Body: "lastHeartbeatTime: \"2024-01-01T02:00:00Z\"\nlastTransitionTime: \"2024-01-01T01:00:00Z\"\nstatus: \"False\"\ntype: Ready\n",
Partial: false,
Requestor: "user-1",
ChangeTime: baseTime.Add(2 * time.Hour),
State: enum.RevisionStateConditionFalse,
},
},
{
name: "No change on LastTransitionTime but changes on LastHeartbeatTime",
condition: &model.K8sResourceStatusCondition{
Type: conditionType,
Status: "False",
LastTransitionTime: baseTime.Add(1 * time.Hour).Format(time.RFC3339),
LastHeartbeatTime: baseTime.Add(3 * time.Hour).Format(time.RFC3339),
},
want: &history.StagingResourceRevision{
Verb: enum.RevisionVerbUpdate,
Body: "lastHeartbeatTime: \"2024-01-01T03:00:00Z\"\nlastTransitionTime: \"2024-01-01T01:00:00Z\"\nstatus: \"False\"\ntype: Ready\n",
Partial: false,
Requestor: "user-1",
ChangeTime: baseTime.Add(3 * time.Hour),
State: enum.RevisionStateConditionFalse,
},
},
{
name: "Condition Removal",
condition: nil,
want: &history.StagingResourceRevision{
Verb: enum.RevisionVerbUpdate,
Body: "",
Partial: false,
Requestor: "user-1",
ChangeTime: baseTime,
State: enum.RevisionStateConditionNotGiven,
},
},
{
name: "Condition Removal (Already Removed)",
condition: nil,
want: nil,
},
},
},
{
name: "Condition Removal",
condition: nil,
want: &history.StagingResourceRevision{
Verb: enum.RevisionVerbUpdate,
Body: "",
Partial: false,
Requestor: "user-1",
ChangeTime: baseTime,
State: enum.RevisionStateConditionNotGiven,
name: "patch conditions without the full status information",
steps: []step{
{
name: "initial patch without status",
condition: &model.K8sResourceStatusCondition{
Type: conditionType,
LastTransitionTime: baseTime.Add(1 * time.Hour).Format(time.RFC3339),
},
want: &history.StagingResourceRevision{
Verb: enum.RevisionVerbUpdate,
Body: "lastTransitionTime: \"2024-01-01T01:00:00Z\"\ntype: Ready\n",
Partial: false,
Requestor: "user-1",
ChangeTime: baseTime.Add(1 * time.Hour),
State: enum.RevisionStateConditionNoAvailableInfo,
},
},
{
name: "patch without status, with heartbeat",
condition: &model.K8sResourceStatusCondition{
Type: conditionType,
LastTransitionTime: baseTime.Add(1 * time.Hour).Format(time.RFC3339),
LastHeartbeatTime: baseTime.Add(2 * time.Hour).Format(time.RFC3339),
},
want: &history.StagingResourceRevision{
Verb: enum.RevisionVerbUpdate,
Body: "lastHeartbeatTime: \"2024-01-01T02:00:00Z\"\nlastTransitionTime: \"2024-01-01T01:00:00Z\"\ntype: Ready\n",
Partial: false,
Requestor: "user-1",
ChangeTime: baseTime.Add(2 * time.Hour),
State: enum.RevisionStateConditionNoAvailableInfo,
},
},
{
name: "patch with status added",
condition: &model.K8sResourceStatusCondition{
Type: conditionType,
LastTransitionTime: baseTime.Add(3 * time.Hour).Format(time.RFC3339),
LastHeartbeatTime: baseTime.Add(2 * time.Hour).Format(time.RFC3339),
Status: "True",
},
want: &history.StagingResourceRevision{
Verb: enum.RevisionVerbUpdate,
Body: "lastHeartbeatTime: \"2024-01-01T02:00:00Z\"\nlastTransitionTime: \"2024-01-01T03:00:00Z\"\nstatus: \"True\"\ntype: Ready\n",
Partial: false,
Requestor: "user-1",
ChangeTime: baseTime.Add(3 * time.Hour),
State: enum.RevisionStateConditionTrue,
},
},
},
},
{
name: "Condition Removal (Already Removed)",
condition: nil,
want: nil,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
l := log.NewLogWithFieldSetsForTest()
cs := history.NewChangeSet(l)
walker.CheckAndRecord(commonFieldSet, k8sFieldSet, tt.condition, cs)
for _, scenario := range scenarios {
t.Run(scenario.name, func(t *testing.T) {
walker := newConditionWalker(parentPath, conditionType)
for _, tt := range scenario.steps {
t.Run(tt.name, func(t *testing.T) {
l := log.NewLogWithFieldSetsForTest()
cs := history.NewChangeSet(l)
walker.CheckAndRecord(commonFieldSet, k8sFieldSet, tt.condition, cs)

if tt.want == nil {
asserter := testchangeset.HasNoRevision{
ResourcePath: resourcepath.Condition(parentPath, conditionType).Path,
}
asserter.Assert(t, cs)
} else {
asserter := testchangeset.HasRevision{
ResourcePath: resourcepath.Condition(parentPath, conditionType).Path,
WantRevision: *tt.want,
}
asserter.Assert(t, cs)
if tt.want == nil {
asserter := testchangeset.HasNoRevision{
ResourcePath: resourcepath.Condition(parentPath, conditionType).Path,
}
asserter.Assert(t, cs)
} else {
asserter := testchangeset.HasRevision{
ResourcePath: resourcepath.Condition(parentPath, conditionType).Path,
WantRevision: *tt.want,
}
asserter.Assert(t, cs)
}
})
}
})
}
Expand Down