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

upgrade google terraform provider #1978

Closed
wants to merge 4 commits into from
Closed

upgrade google terraform provider #1978

wants to merge 4 commits into from

Conversation

trajan0x
Copy link
Contributor

@trajan0x trajan0x commented Feb 5, 2024

Description

As part of golang/go#65363 attempt to upgrade terraform-provider-google. This involves some unsafe ops, may revert later

Summary by CodeRabbit

  • Refactor
    • Updated configuration handling across multiple Terraform providers to utilize transport.Config instead of google.Config, enhancing compatibility and code maintainability.
    • Introduced utility functions for token source retrieval and accessing unexported fields, improving code efficiency and flexibility.
  • New Features
    • Enhanced context handling in data sources with additional package imports and reflection usage, allowing for more robust data processing and integration capabilities.

@trajan0x trajan0x requested a review from ChiTimesChi as a code owner February 5, 2024 21:08
@github-actions github-actions bot added go Pull requests that update Go code size/l labels Feb 5, 2024
Copy link
Contributor

coderabbitai bot commented Feb 5, 2024

Important

Auto Review Skipped

Review was skipped due to path filters

Files ignored due to path filters (26)
  • contrib/git-changes-action/go.mod is excluded by: !**/*.mod
  • contrib/promexporter/go.mod is excluded by: !**/*.mod
  • contrib/terraform-provider-helmproxy/go.mod is excluded by: !**/*.mod
  • contrib/terraform-provider-iap/go.mod is excluded by: !**/*.mod
  • contrib/terraform-provider-kubeproxy/go.mod is excluded by: !**/*.mod
  • core/go.mod is excluded by: !**/*.mod
  • core/go.sum is excluded by: !**/*.sum
  • ethergo/go.mod is excluded by: !**/*.mod
  • ethergo/go.sum is excluded by: !**/*.sum
  • go.work.sum is excluded by: !**/*.sum
  • services/cctp-relayer/go.mod is excluded by: !**/*.mod
  • services/cctp-relayer/go.sum is excluded by: !**/*.sum
  • services/explorer/go.mod is excluded by: !**/*.mod
  • services/explorer/go.sum is excluded by: !**/*.sum
  • services/omnirpc/go.mod is excluded by: !**/*.mod
  • services/omnirpc/go.sum is excluded by: !**/*.sum
  • services/rfq/go.mod is excluded by: !**/*.mod
  • services/rfq/go.sum is excluded by: !**/*.sum
  • services/scribe/go.mod is excluded by: !**/*.mod
  • services/scribe/go.sum is excluded by: !**/*.sum
  • services/sinner/go.mod is excluded by: !**/*.mod
  • services/sinner/go.sum is excluded by: !**/*.sum
  • services/stiprelayer/go.mod is excluded by: !**/*.mod
  • services/stiprelayer/go.sum is excluded by: !**/*.sum
  • tools/go.mod is excluded by: !**/*.mod
  • tools/go.sum is excluded by: !**/*.sum

Walkthrough

The overall change across various terraform-provider modules involves a significant shift from using google.Config to transport.Config for configuration handling. This update reflects a broader move towards standardizing on the transport.Config structure across different providers. Additionally, there's an introduction of new utility functions for token source retrieval and accessing unexported fields via reflection, enhancing the flexibility and capability of the providers in handling configurations and contexts.

Changes

Files Summary
.../helmproxy/provider/provider.go
.../kubeproxy/provider/main_provider.go
.../kubeproxy/provider/manifest_provider.go
Replaced google.Config with transport.Config affecting type declarations and assignments.
.../iap/provider/iap-tunnel.go
.../tfcore/utils/tunnel.go
.../tfcore/utils/utils.go
Introduced transport.Config and utility functions for token handling and reflection.
.../iap/provider/keep_alive.go Imported additional packages and updated context handling and package imports.

🐇✨
In the land of code and byte,
Where configs shift from day to night,
A rabbit hops with glee and might,
For transport's path has found the light.
🌟🐾

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository from git and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between d6f67a0 and 7ba1564.
Files ignored due to path filters (17)
  • agents/go.mod is excluded by: !**/*.mod
  • agents/go.sum is excluded by: !**/*.sum
  • contrib/git-changes-action/go.mod is excluded by: !**/*.mod
  • contrib/git-changes-action/go.sum is excluded by: !**/*.sum
  • contrib/promexporter/go.mod is excluded by: !**/*.mod
  • contrib/promexporter/go.sum is excluded by: !**/*.sum
  • contrib/release-copier-action/go.mod is excluded by: !**/*.mod
  • contrib/release-copier-action/go.sum is excluded by: !**/*.sum
  • contrib/screener-api/go.mod is excluded by: !**/*.mod
  • contrib/screener-api/go.sum is excluded by: !**/*.sum
  • contrib/tfcore/generated/google/doc.go is excluded by: !**/generated/**
  • contrib/tfcore/generated/google/generate.go is excluded by: !**/generated/**
  • contrib/tfcore/generated/google/getters.go is excluded by: !**/generated/**
  • contrib/tfcore/generated/google/google_gen.go is excluded by: !**/generated/**
  • contrib/tfcore/go.mod is excluded by: !**/*.mod
  • contrib/tfcore/go.sum is excluded by: !**/*.sum
  • go.work.sum is excluded by: !**/*.sum
Files selected for processing (8)
  • contrib/terraform-provider-helmproxy/provider/provider.go (2 hunks)
  • contrib/terraform-provider-iap/provider/iap-tunnel.go (3 hunks)
  • contrib/terraform-provider-iap/provider/keep_alive.go (3 hunks)
  • contrib/terraform-provider-iap/provider/provider.go (1 hunks)
  • contrib/terraform-provider-kubeproxy/provider/main_provider.go (2 hunks)
  • contrib/terraform-provider-kubeproxy/provider/manifest_provider.go (2 hunks)
  • contrib/tfcore/utils/tunnel.go (3 hunks)
  • contrib/tfcore/utils/utils.go (1 hunks)
Additional comments: 6
contrib/terraform-provider-iap/provider/provider.go (1)
  • 6-6: Ensure the updated import path github.com/hashicorp/terraform-provider-google/google/provider is correct and compatible with the rest of the codebase.
contrib/terraform-provider-helmproxy/provider/provider.go (1)
  • 18-18: Confirm that the replacement of google.Config with transport.Config is correctly implemented and compatible with the provider's configuration logic.
contrib/terraform-provider-kubeproxy/provider/main_provider.go (1)
  • 16-16: Ensure that the transition to transport.Config from google.Config is correctly handled and does not affect the provider's functionality adversely.
contrib/tfcore/utils/tunnel.go (1)
  • 18-18: Confirm that the change of the config parameter type to *transport.Config in the StartTunnel function is correctly implemented and integrated.
contrib/terraform-provider-iap/provider/iap-tunnel.go (1)
  • 89-89: Ensure that the switch to transport.Config and the use of utils.GetTokenSource for token source assignment in dataSourceProxy function are correctly implemented.
contrib/terraform-provider-kubeproxy/provider/manifest_provider.go (1)
  • 195-195: Confirm that the type assertion to *transport.Config in the googConfigureContextFunc function is correctly implemented and does not introduce any compatibility issues.

Comment on lines +10 to +18
func GetTokenSource(config *transport.Config) oauth2.TokenSource {
cfgReflection := reflect.ValueOf(config)
accessToken := GetUnexportedField(cfgReflection.FieldByName("tokenSource")).(oauth2.TokenSource)
return accessToken
}

func GetUnexportedField(field reflect.Value) interface{} {
return reflect.NewAt(field.Type(), unsafe.Pointer(field.UnsafeAddr())).Elem().Interface()
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using reflection and the unsafe package to access unexported fields can lead to maintenance challenges and potential runtime issues. Confirm the necessity of these approaches and ensure their safe use.

Comment on lines +74 to +76
cfgReflection := reflect.ValueOf(config)
context := utils.GetUnexportedField(cfgReflection.FieldByName("context")).(context2.Context)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of reflection to handle context in dataSourceKeepAlive function raises concerns about potential side effects. Verify the necessity and correctness of this approach.

@trajan0x trajan0x changed the title attempt to fix the tunnel upgrade google terraform provider Feb 5, 2024
Copy link

codecov bot commented Feb 5, 2024

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (8c9e8f7) 50.41387% compared to head (1cd3904) 42.68385%.
Report is 115 commits behind head on master.

Files Patch % Lines
contrib/tfcore/utils/utils.go 0.00000% 6 Missing ⚠️
contrib/tfcore/utils/tunnel.go 0.00000% 2 Missing ⚠️
Additional details and impacted files
@@                 Coverage Diff                 @@
##              master       #1978         +/-   ##
===================================================
- Coverage   50.41387%   42.68385%   -7.73003%     
===================================================
  Files            404         124        -280     
  Lines          28028        9233      -18795     
  Branches         307           0        -307     
===================================================
- Hits           14130        3941      -10189     
+ Misses         12509        4828       -7681     
+ Partials        1389         464        -925     
Flag Coverage Δ
agents ?
core 64.38188% <ø> (+0.12238%) ⬆️
ethergo ?
explorer 24.80122% <ø> (ø)
git-changes-action 53.94265% <ø> (ø)
omnirpc ?
packages ?
promexporter 74.80916% <ø> (?)
rfq ?
screener-api 66.27451% <ø> (ø)
scribe 52.21130% <ø> (ø)
sinner ?
solidity ?
terraform-provider-helmproxy ?
terraform-provider-iap ?
terraform-provider-kubeproxy ?
tfcore 29.26045% <0.00000%> (-0.57563%) ⬇️
tools ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Feb 20, 2024
@github-actions github-actions bot closed this Feb 25, 2024
@github-actions github-actions bot deleted the fix/tunnel branch May 9, 2024 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant