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

Enables Running BYOC in Containerized Environments/TFC #183

Merged
merged 5 commits into from
Dec 12, 2024
Merged

Conversation

gene-redpanda
Copy link
Member

@gene-redpanda gene-redpanda commented Nov 6, 2024

Fixes various issues that prevented running the provider in a containerized environment including various cred passthrus and getting the Azure CLI removed as a dependency for the Azure flow.

Also correctly deserializes GRPC errors and reports the user-agent back home.

@gene-redpanda gene-redpanda force-pushed the byoc-ci branch 3 times, most recently from cbd4aa7 to c2db864 Compare November 7, 2024 21:28
@gene-redpanda gene-redpanda force-pushed the byoc-ci branch 3 times, most recently from cf110ce to 279b40d Compare November 15, 2024 21:38
Adds support for testing byoc
Adds initial error handling improvements
Adds support for useragent reporting
Also includes more error handling improvement
@gene-redpanda gene-redpanda changed the title Add CI for BYOC Enables Running BYOC in Containerized Environments/TFC Dec 12, 2024
@gene-redpanda gene-redpanda marked this pull request as ready for review December 12, 2024 15:50
Copy link

@RafalKorepta RafalKorepta left a comment

Choose a reason for hiding this comment

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

I don't see any blockers! Good job!

Comment on lines +30 to +34
# aws_private_link = {
# enabled = true
# connect_console = true
# allowed_principals = ["arn:aws:iam::123456789024:root"]
# }

Choose a reason for hiding this comment

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

NIT: ✂️

Copy link
Member Author

Choose a reason for hiding this comment

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

It's solely there for an example to end users. We can't have it actually run because it will cause the whole thing to explode

Comment on lines +30 to +34
# azure_private_link = {
# enabled = true
# connect_console = true
# allowed_subscriptions = ["12345678-1234-1234-1234-123456789012"]
# }

Choose a reason for hiding this comment

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

NIT: ✂️

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above

@@ -171,7 +175,7 @@ func SpawnConn(url, authToken string) (*grpc.ClientConn, error) {
rl.Limiter,
// Retry interceptor
grpcretry.UnaryClientInterceptor(
grpcretry.WithCodes(codes.Unavailable, codes.Unknown, codes.Internal),
grpcretry.WithCodes(codes.Unavailable, codes.Unknown, codes.Internal, codes.Unauthenticated),

Choose a reason for hiding this comment

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

Not a blocker: Could the CI uses the credentials from the secret manager that would authenticate against our cloud?

I think it should be under sdlc/prod/buildkite/rpk_test_client

Copy link
Member Author

Choose a reason for hiding this comment

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

I've got creds and vaults set up for secret manager that are per repo across several repos. I'd prefer to not collide with other testing/deal with potential quotas 😅

Comment on lines +292 to +293
googleCredentials := os.Getenv("GOOGLE_CREDENTIALS")
googleCredentialsBase64 := os.Getenv("GOOGLE_CREDENTIALS_BASE64")

Choose a reason for hiding this comment

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

Not a blocker: I always wonder if there is base64 why should we use the other one? Maybe you know.

Copy link
Member Author

Choose a reason for hiding this comment

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

IMO we shouldn't. I support the standard because I want to avoid issues for end users but IMO the base64 should absolutely be the default.

"github.com/redpanda-data/terraform-provider-redpanda/redpanda/validators"
)

func resourceClusterSchema() schema.Schema {

Choose a reason for hiding this comment

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

Is this generated or hand crafted?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hand crafted. There's unfortunately no up to date generator for TPF (sdkv2 has a couple).

@gene-redpanda gene-redpanda merged commit 9fdd364 into main Dec 12, 2024
5 checks passed
@gene-redpanda gene-redpanda deleted the byoc-ci branch December 12, 2024 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants