fix: hash the real content when ConfigMap data is not a map - #6262
fix: hash the real content when ConfigMap data is not a map#6262PragalvaXFREZ wants to merge 1 commit into
Conversation
kubernetes-prow
Bot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: PragalvaXFREZ The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @PragalvaXFREZ! |
|
Hi @PragalvaXFREZ. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
kubernetes-prow
Bot
added
needs-ok-to-test
getNodeValues collects the fields that feed the ConfigMap/Secret name
suffix hash. For any non-scalar field it marshals the node to JSON and
decodes it into a map[string]interface{}, discarding the decode error:
// data, binaryData and stringData are all maps
var v map[string]interface{}
json.Unmarshal(vs, &v)
The comment holds for the value kustomize generates itself, but a
patches entry can replace data with any JSON value. When it is not an
object the decode fails, v stays nil, and the hash is computed over
"data": null. Every ConfigMap in that shape hashes to the same suffix
no matter what it contains.
Two builds differing only in content, using a JSON 6902 patch that
replaces /data with a sequence:
data:
- A -> name: cm-dk855m5d49
data:
- B -> name: cm-dk855m5d49
The same pair with an object value is hashed correctly, which shows the
hash is computed after patches over the final content:
data: {k: A} -> name: cm-c74h26c25g
data: {k: B} -> name: cm-t6kk6cfb8c
The suffix exists so that changed content produces a new object name.
Here the content changes and the name does not, so a rollout is silently
skipped and two such ConfigMaps applied to one namespace collide.
Decoding into interface{} keeps every hash that is correct today byte
identical, since a JSON object still decodes to map[string]interface{}
and the values["binaryData"].(map[string]interface{}) assertion in
encodeConfigMap is unaffected. Only the broken non-object case changes.
Confirmed by diffing kustomize build output from both binaries across
ConfigMap (plain, with binaryData, empty) and Secret (Opaque,
kubernetes.io/tls) generators: identical.
After the fix the two sequence cases hash to cm-9k92ch6bch and
cm-td45tkc82d.
The added test fails on master with:
different data hashed identically: "dk855m5d49"
Signed-off-by: Pragalva Sapkota <sapkotapragalva@gmail.com>
737a3d8 to
4d74d98
Compare
1 participant
getNodeValuescollects the fields that feed the ConfigMap/Secret name suffix hash. For any non-scalar field it marshals the node to JSON and decodes it into amap[string]interface{}, discarding the decode error:The comment holds for the value kustomize generates itself, but a
patchesentry can replacedatawith any JSON value. When it is not an object the decode fails,vstays nil, and the hash is computed over"data": null. Every ConfigMap in that shape hashes to the same suffix no matter what it contains.Two builds that differ only in content, using a JSON 6902 patch that replaces
/datawith a sequence:The same pair with an object value behaves correctly, which shows the hash is computed after patches over the final content rather than over the pre-patch value:
The suffix exists so that changed content produces a new object name. Here the content changes and the name does not, so a rollout is silently skipped, and two such ConfigMaps applied to one namespace collide.
Decoding into
interface{}keeps every hash that is correct today byte identical, since a JSON object still decodes tomap[string]interface{}andencodeConfigMap'svalues["binaryData"].(map[string]interface{})assertion is unaffected. Only the currently broken non-object case changes. Confirmed by diffingkustomize buildoutput from both binaries across ConfigMap (plain, with binaryData, empty) and Secret (Opaque, kubernetes.io/tls) generators: identical.After the fix the two sequence cases hash to
cm-9k92ch6bchandcm-td45tkc82d.Adds a regression test to
hasher_test.go; it fails on master withdifferent data hashed identically: "dk855m5d49".TestConfigMapHashandTestSecretHashstill pass, and theapimodule suite shows no new failures.