Skip to content

[Fix] Clear stale address/resource_name when a re-collected asset longer has them#118

Open
Center-Sun wants to merge 1 commit into
antgroup:mainfrom
Center-Sun:fix/stale-resource-fields-on-update
Open

[Fix] Clear stale address/resource_name when a re-collected asset longer has them#118
Center-Sun wants to merge 1 commit into
antgroup:mainfrom
Center-Sun:fix/stale-resource-fields-on-update

Conversation

@Center-Sun

Copy link
Copy Markdown
Contributor

Thank you for your contribution to CloudRec!

What About:

  • Server (java)
  • Collector (go)
  • Rule (opa)

Description:

updateByPrimaryKeySelective guarded every column with , but the collector push is a full snapshot: when a derived field such as the public IP behind RowField.Address disappears upstream, the collector omits it (omitempty), the server receives null, and the selective guard silently skips the column. The instance JSON is overwritten with the new snapshot while the address column keeps the released IP forever - the value->empty transition is unrepresentable.

Write address and resource_name unconditionally, following the existing deleted_at pattern in the same statement (already unconditional so pre-delete tags can be cleared). The sole caller of this statement is the collector snapshot persistence path (SaveResourceServiceImpl.saveOrUpdateData), which always populates the PO from the pushed payload, so no other write path is affected. Stale rows self-heal on the next collection cycle.

…longer has them

updateByPrimaryKeySelective guarded every column with <if test="... != null">,
but the collector push is a full snapshot: when a derived field such as the
public IP behind RowField.Address disappears upstream, the collector omits it
(omitempty), the server receives null, and the selective guard silently skips
the column. The instance JSON is overwritten with the new snapshot while the
address column keeps the released IP forever - the value->empty transition is
unrepresentable.

Write address and resource_name unconditionally, following the existing
deleted_at pattern in the same statement (already unconditional so pre-delete
tags can be cleared). The sole caller of this statement is the collector
snapshot persistence path (SaveResourceServiceImpl.saveOrUpdateData), which
always populates the PO from the pushed payload, so no other write path is
affected. Stale rows self-heal on the next collection cycle.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request modifies the MyBatis mapper to unconditionally update the address and resource_name fields, aiming to clear stale values when they disappear upstream. However, doing so within a selective update query breaks its contract and risks silent data corruption if partially populated objects are updated. It is recommended to keep the selective update method truly selective and instead use a standard non-selective update method for this purpose.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +179 to +186
<!-- address/resource_name mirror the latest collector snapshot: write unconditionally
(like deleted_at) so a value that disappears upstream, e.g. a released public IP,
is cleared instead of leaving a stale value behind -->
address = #{address,jdbcType=VARCHAR},
<if test="resourceId != null">
resource_id = #{resourceId,jdbcType=VARCHAR},
</if>
<if test="resourceName != null">
resource_name = #{resourceName,jdbcType=VARCHAR},
</if>
resource_name = #{resourceName,jdbcType=VARCHAR},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Description of the Issue

Modifying updateByPrimaryKeySelective to unconditionally update address and resource_name (similar to the existing deleted_at field) breaks the fundamental contract of a selective update.

If any other component or future feature calls updateByPrimaryKeySelective with a partially populated CloudResourceInstancePO (for example, to update only version or delNum), it will unexpectedly overwrite address, resource_name, and deleted_at with NULL in the database. This introduces a significant risk of silent data corruption.

Suggested Solution

To safely support clearing these fields while preserving the selective update contract:

  1. Keep updateByPrimaryKeySelective truly selective by retaining the <if test="... != null"> guards for address, resource_name, and deleted_at.
  2. Introduce a standard updateByPrimaryKey method in CloudResourceInstanceMapper (both the Java interface and XML mapper) that updates all fields unconditionally.
  3. Use updateByPrimaryKey in SaveResourceServiceImpl: Since saveOrUpdateData already loads the full PO from the database via findOne before updating, it is fully populated. Calling updateByPrimaryKey there is perfectly safe and will correctly set address and resource_name to NULL if they are missing from the latest collector snapshot.

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.

1 participant