Repository navigation
Conversation
|
Thanks for the approval, @vjancich! CI is clean and this simply restores broker log delivery now that AWS has GA'd it for MSK Express brokers. @bryantbiggs / @antonbabenko — would you be able to merge when you have a moment? Thanks! |
|
Gentle ping — this has been green and community-approved (thanks @vjancich) for about a month now. It is a small fix: broker log delivery was silently skipped for Express brokers. Happy to rebase or split it further if that helps it land. No rush if the queue is deep. |
|
what provider version enabled this - because it wasn't always possible. if we don't raise the min supported provider version for where it was enabled, we break workflows. small fixes are rarely simple |
|
Fair question — I dug into it, and the short answer is that no provider version enabled this, because the provider never gated it. The block was the Kafka API itself, so there is no min-version to raise to. Details: What actually blocked it. The error that motivated the guard in #48 is quoted in #47:
That is an AWS API response, not a Terraform schema or validation error. AWS lifted it on 2026-02-11: Amazon MSK now supports broker logs on Express Brokers ("available for both new and existing Express brokers… in all AWS regions where express brokers are available"). The developer guide now states it directly: "Broker logs are available on both MSK Standard and Express brokers" (msk-logging). Why the provider isn't a factor.
And even on the most conservative reading, this module already requires One upgrade note that is worth flagging, since it is the closest thing to a real behavior change here: an existing Express-broker user who upgrades will start seeing |
|
Any updates on this? It would really help to have those logs now that AWS supports it. |
|
@bryantbiggs following up on the provider-version question. The short version of my July reply: no provider version gated this. The restriction was on the AWS API side ( If you would still prefer a floor on the provider version, say which one and I will add it. CI is green and @vjancich approved in July. @danielOfir1 is running into the same gap. |
|
There has to be a provider version bump for when this was enabled - the Go AWS SDK v2 would have to have changed to allow this and the AWS provider consumes this and is released at some version |
|
I went and checked the SDK, because that premise is testable. It turns out no SDK change was needed, and none happened — the wire model could always express this. 1. In // The type of Amazon EC2 instances to use for Apache Kafka brokers...
//
// This member is required.
InstanceType *stringIt's a 2. LoggingInfo *types.LoggingInfoA plain optional field. There is no relationship in the model between it and So the SDK could always serialize "Express instance type + 3. The provider resource has no Express handling either.
So there is no provider release to raise the floor to, because no provider release changed behaviour here. The module already requires That said, I take the underlying concern seriously: you don't want an upgrade to silently break someone. The real upgrade risk here isn't a provider version, it's the plan diff — an existing Express user will start seeing a If that bothers you, I'm happy to push the guard I offered in July: dynamic "logging_info" {
for_each = var.cloudwatch_logs_enabled || var.firehose_logs_enabled || var.s3_logs_enabled ? [true] : []
...
}That keeps the block absent until a sink is actually enabled, so no existing user sees a diff at all — and it drops the instance-type coupling that caused #47 in the first place. Say the word and I'll push it. |
Description
Express brokers now support broker log delivery (CloudWatch Logs, Firehose, S3), but the module still restricts the
logging_infoblock to Standard brokers, socloudwatch_logs_enabled/firehose_logs_enabled/s3_logs_enabledare silently ignored on Express clusters.The Express guard was introduced in #48, back when Express brokers did not accept a
logging_infoblock. AWS has since added broker log delivery for Express brokers (https://docs.aws.amazon.com/msk/latest/developerguide/msk-logging.html), so the guard is no longer needed. This reverts thedynamic "logging_info"wrapper back to the original staticlogging_infoblock, applying the same logging configuration to both broker types.Closes #68
Validation
terraform validate-> Success! The configuration is valid.pre-commit run --files main.tf-> terraform_fmt, terraform_docs, terraform_validate, terraform_validate with tflint all pass.Breaking Changes
None for Standard brokers (the rendered
logging_infoblock is unchanged). Express brokers gain the logging configuration that was previously ignored.