-
Notifications
You must be signed in to change notification settings - Fork 230
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
Fix double base64-encoding of ca_bundle
field
#1296
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
0679629
to
ffded33
Compare
nicholasSUSE
approved these changes
Jan 26, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approved, but please wait for @rohitsakala review before merging
@@ -26,7 +29,8 @@ func flattenCatalogV2(d *schema.ResourceData, in *ClusterRepo) error { | |||
} | |||
d.Set("resource_version", in.ObjectMeta.ResourceVersion) | |||
|
|||
d.Set("ca_bundle", string(in.Spec.CABundle)) | |||
encodedCABundle := base64.StdEncoding.EncodeToString(in.Spec.CABundle) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's test the following:
output "some_NAME" {
value = <the_catalog_resource>.ca_bundle
}
to validate this
rohitsakala
approved these changes
Jan 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue:
For #1297
Problem
The user reports that configuring a
rancher_catalog_v2
resource with theca_bundle
field set fails. This is because thecaBundle
field in the configuredClusterRepo
resource is base64-encoded twice. For more details, the JIRA issue gives a pretty good description.Also note that there is a discrepancy between the documentation for configuring a repo/catalog via the Rancher UI, and the documentation for the
ca_bundle
field in therancher_catalog_v2
resource:Solution
The cleanest solution, as far as @nicholasSUSE and I can tell, is to slightly change the behavior of the
ca_bundle
field so that the user sets it to a base64-encoded DER value. When expanding the terraform object into aClusterRepo
, therancher2
provider base64-decodes the passedca_bundle
value and sets thespec.caBundle
field of theClusterRepo
to the result, which is of type[]byte
. When theencoding/json
package marshals theClusterRepo
struct created by therancher2
provider, it automatically base64-encodes thecaBundle
field (see the docs forjson.Marshal
), which appears to be what the Rancher API expects.Testing
This issue can be reproduced by running the terraform provider against a v2.7.6 Rancher cluster. Note that running it against a version of Rancher run via
go run main.go
ordlv
doesn't work - it seems that the terraform provider chokes on the fact that something (I never figured out what) has the versiondev
.Engineering Testing
Manual Testing
On a Rancher v2.7.6 deployment running in k3d, I did the following tests using the terraform provider:
ClusterRepo
resource with thecaBundle
field set.ClusterRepo
resource without thecaBundle
field set.ClusterRepo
resource.ClusterRepo
resource by changing thename
field in the terraform resource. This failed, but I determined that this failure is present on themaster
branch and thus not related to this PR. This seems wrong to me, but also seems like it may just be due to how the Rancher API works. I'm new to the Rancher project, so can someone who has more experience with Rancher check this?Automated Testing
I updated the
ca_bundle
field in theexpandCatalogV2
andflattenCatalogV2
unit tests to reflect the HCL value being base64-encoded DER, and the ClusterRepo value being a[]byte
containing unencoded DER.QA Testing Considerations
I don't think there is any issue with changing the behavior of the
ca_bundle
field, since it is broken as is. I could easily be wrong though...Regressions Considerations