Skip to content
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.

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

Update istio integration tests to account for removal of cacert field from config dump #50043

Merged

Conversation

MorrisLaw
Copy link
Contributor

@MorrisLaw MorrisLaw commented Mar 21, 2024

Please provide a description of this PR:

This PR addresses the istio integration failures noticed here: #49964 (comment)

This was a result of the recent changes to migrate to rustls where a struct was modified in ztunnel and removed a field (ca_cert) that gets checked for an istio integration test called TestIntermediateCertRefresh.

Here is the struct in it's current state:

pub struct CertsDump {
    identity: String,
    state: String,
    cert_chain: Vec<CertDump>,
}

instead of:

pub struct CertsDump {
    identity: String,
    state: String,
    ca_cert: Vec<CertDump>,
    cert_chain: Vec<CertDump>,
}

This failure is preventing ztunnel from getting updated in istio.deps because the integration tests are failing. This should address the issue by pointing to the ca cert that resides in the cert chain as opposed to pointing to a field that no longer exists in this updated CertsDump struct.

@MorrisLaw MorrisLaw requested review from a team as code owners March 21, 2024 20:37
@istio-policy-bot istio-policy-bot added the area/ambient Issues related to ambient mesh label Mar 21, 2024
@istio-testing istio-testing added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 21, 2024
@MorrisLaw MorrisLaw added the release-notes-none Indicates a PR that does not require release notes. label Mar 21, 2024
@MorrisLaw
Copy link
Contributor Author

/retest

@keithmattix
Copy link
Contributor

Looks like the issue is the secret dump formatter is not matching the golden file.

@istio-testing istio-testing added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 22, 2024
@istio-testing istio-testing merged commit 6e34ef5 into istio:master Mar 22, 2024
28 checks passed
@ilrudie ilrudie mentioned this pull request Mar 22, 2024
jmcshane pushed a commit to superorbital/istio that referenced this pull request Apr 3, 2024
… from config dump (istio#50043)

* Update istio integration tests to account for removal of ca_crt field from config dump

* use the latest ztunnel master sha

* update secret printing and testdata file to have no ca_cert field
@MorrisLaw MorrisLaw deleted the account-for-new-rustls-migration branch April 4, 2024 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ambient Issues related to ambient mesh release-notes-none Indicates a PR that does not require release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants