Replacing sensitive values doesn't work in certain cases.

[klaudworks]

What happened?

After implementing #83, I manually tested the implementation using the request in examples/sample/request.yaml. It didn't work anymore while the example reconciles successfully on main.

I noticed that the HTTP response in the status is stored as clear text instead of masking sensitive values after successful reconciling.

When digging deeper I noticed the following happens:

1. the first user is created via a POST request and the values are masked and stored in the custom resource:

   at this point cr.response.status.body.data look as they should look:

   "{\"age\": {{response-user-password:default:extracted-user-age}}, \"email\": \"{{response-secret:default:extracted-user-email}}\", \"password\": \"{{response-user-password:default:extracted-user-password}}\", \"username\": \"mock_user\", \"id\": \"13\"}"
   

2. in the observe step, we need to access the masked values to assemble our GET request

         # Scenario 2: Action specified, method not specified (defaults to GET for OBSERVE)
         - action: OBSERVE
           # method: "GET"
           url: (.payload.baseUrl + "/" + (.response.body.id|tostring)) # .response.body.id is null
   

3. the response body is null because the JSON in step 1. can not be parsed into an object via the function GenerateRequestObject. I assume this is during the age field being masked and not quoted as a string. This leads to the function parsing the whole response status as a single string instead of a nested object.

   // GenerateRequestObject creates a JSON-compatible map from the specified Request's ForProvider and Response fields.
   // It merges the two maps, converts JSON strings to nested maps, and returns the resulting map.
   func GenerateRequestObject(forProvider v1alpha2.RequestParameters, response v1alpha2.Response) map[string]interface{} {
   	baseMap, _ := json_util.StructToMap(forProvider)
   	statusMap, _ := json_util.StructToMap(map[string]interface{}{
   		"response": response,
   	})
   
   	maps.Copy(baseMap, statusMap)
   	json_util.ConvertJSONStringsToMaps(&baseMap)
   
   	return baseMap
   }
   

4. When we request http://flask-api.default.svc.cluster.local/v1/users/null the response is 404. The sample request is configured to view a 404 as the object does not exist. Therefore it tries to create it again.

5. This time the secrets to store the user already exist. Therefore, the function updateSecretWithPatchedValue returns early and therefore does not execute the function replaceSensitiveValues.

6. Given, that the values in cr.response.body.data are not masked anymore, it can be successfully parsed as a json and the observe request can be successfully assembled this time. Therefore reconciliation succeeds on the second time.

func updateSecretWithPatchedValue(ctx context.Context, kubeClient client.Client, logger logging.Logger, data *httpClient.HttpResponse, secret *corev1.Secret, secretKey string, requestFieldPath string) error {
	// Step 1: Parse and prepare data
	dataMap, err := prepareDataMap(data)
	if err != nil {
		return err
	}

	// Step 2: Extract the value to patch
	valueToPatch := extractValueToPatch(logger, dataMap, requestFieldPath)

	// Step 3: Check if the value is already present
	if isSecretDataUpToDate(secret, secretKey, valueToPatch) { # reconciliation is skipped the second time
		return nil
	}

	// Step 4: Update the secret data
	updateSecretData(secret, secretKey, valueToPatch)

	// Step 5: Replace sensitive values in the HTTP response
	replaceSensitiveValues(data, secret, secretKey, valueToPatch)

	// Step 5: Save the updated secret to the Kubernetes API
	return kubehandler.UpdateSecret(ctx, kubeClient, secret)
} 

Uncovered Problems

The behavior above is due to a combination of the following problems:

1. Numbers can't be masked

Whenever a number is masked while parsing the creation response, the following observation step fails because masked values can only be parsed if they are strings.

E.g. this is not valid JSON unless you replace the placeholder for age.

"{\"age\": {{response-user-password:default:extracted-user-age}}, \"email\": \"{{response-secret:default:extracted-user-email}}\", \"password\": \"{{response-user-password:default:extracted-user-password}}\", \"username\": \"mock_user\", \"id\": \"13\"}"

2. Referencing secret values

It's not possible to reference masked values in a response because you will reference the placeholder instead.

3. Masking does not work when the secrets already exist

If the secret already exist, the masking of the response values is just skipped.

Solution to merge #83

We can't merge my PR to add a feature for missingFieldStrategy right now because it implicitly fixes problem number 3. When, the masking works properly, then the e2e test doesn't run anymore.

My proposed solution would be to extend PR #83 to replace all secret values before parsing the cr.status.response.body into an object. This would fix all 3 problems.

I'd prefer to just extend the existing PR because it already touches the secret patching procedure. @arielsepton would that be fine for you?

[arielsepton]

Thanks for the detailed breakdown — it all makes sense. Extending your PR sounds good to me. Feel free to proceed, and let me know if you need anything from me!

Appreciate the effort on this!