Fix bugs in googleca and update flag description

[priyawadhwa]

This fixes 2 bugs in the googleca:

1. We would always fail in TrustBundle because we were checking if len(roots) == 0 instead of if len(caCerts) == 0, roots was always empty at this point
2. Allow specifying certificateAuthority in the gcp_private_ca_parent flag

Signed-off-by: Priya Wadhwa [email protected]

Release Note

Fix bugs in googleca and update flag description

[haydentherapper]

just a couple comments, looks good!

[haydentherapper]

I'd merge these two variables into one, caPool, since they represent the same resource. You can then check if certAuthority is set to decide whether or not to target a specific CA

[priyawadhwa]

So we do need both of them later on, the full certAuthorityParent in the GetCertificateAuthority request and just the name of the CA certAuthority when actually making the request for IssuingCertificateAuthorityId.

i think it's more understandable to do all the validation/string splitting logic at the beginning and then have everything we need later on when actually getting CA's/making requests.

[haydentherapper]

Oh I see my confusion, it's around terminology. I thought two referred to the same resource. I would use the terms from aip.dev to describe the resources and IDs here:

• certAuthority -> certAuthorityID (since it represents just an ID)
• certAuthorityParent -> certAuthorityResource (since it's the full resource string)
• caPoolParent -> caPoolResource

Does that sound good? "parent" was what was confusing, since a parent of the a resource is a different resource.

[priyawadhwa]

Sure, that's fine. I'd still keep "parent" personally to follow the google naming standards for a googleca, but am fine with changing it too. It's updated now.

[codecov-commenter]

Codecov Report

Merging #897 (c0981b3) into main (81ecec8) will decrease coverage by 0.84%.
The diff coverage is 17.54%.

@@            Coverage Diff             @@
##             main     #897      +/-   ##
==========================================
- Coverage   55.63%   54.78%   -0.85%     
==========================================
  Files          38       38              
  Lines        2333     2380      +47     
==========================================
+ Hits         1298     1304       +6     
- Misses        939      980      +41     
  Partials       96       96              

Impacted Files	Coverage Δ
pkg/ca/googleca/v1/googleca.go	37.50% <14.54%> (-10.05%)	⬇️
cmd/app/serve.go	19.90% <100.00%> (+0.37%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.