New Resource - azurerm_log_analytics_workspace_table_custom_log#30800
New Resource - azurerm_log_analytics_workspace_table_custom_log#30800sreallymatt merged 23 commits intomainfrom
azurerm_log_analytics_workspace_table_custom_log#30800Conversation
internal/services/loganalytics/log_analytics_workspace_table_custom_log_resource.go
Outdated
Show resolved
Hide resolved
internal/services/loganalytics/log_analytics_workspace_table_custom_log_resource.go
Outdated
Show resolved
Hide resolved
internal/services/loganalytics/log_analytics_workspace_table_custom_log_resource.go
Outdated
Show resolved
Hide resolved
|
|
||
| "type": { | ||
| Type: pluginsdk.TypeString, | ||
| Required: true, |
There was a problem hiding this comment.
Since this is Required, you should move it above any optional properties in the schema
| Plan: pointer.To(tables.TablePlanEnum(config.Plan)), | ||
| Schema: &tables.Schema{ | ||
| Categories: pointer.To(config.Categories), | ||
| Columns: expandColumns(&config.Columns), |
There was a problem hiding this comment.
Do we have to pass the address here?
There was a problem hiding this comment.
Nope! Changed to take the struct instead of pointer.
|
|
||
| if model := resp.Model; model != nil { | ||
| if props := model.Properties; props != nil { | ||
| if pointer.From(props.Plan) == tables.TablePlanEnumAnalytics { |
There was a problem hiding this comment.
I'm confused about this whole check. Can you give some context for why this is here
There was a problem hiding this comment.
I tried to clarify with variable naming. Better?
| state.Solutions = pointer.From(props.Schema.Solutions) | ||
|
|
||
| // The `Categories` property is not retrievable, we attempt to obtain from config. Imports will need `ignore_changes`. | ||
| if props.Schema.Categories != nil { |
There was a problem hiding this comment.
Having an if that doesn't do anything past the check seems odd. I'm for removing categories altogether but something to keep in mind if you see something like this in the future
internal/services/loganalytics/log_analytics_workspace_table_custom_log_resource.go
Show resolved
Hide resolved
| }, | ||
| }, | ||
|
|
||
| "retention_in_days": { |
|
|
| Type: pointer.To(tables.ColumnTypeEnum(column.Type)), | ||
| DataTypeHint: pointer.To(tables.ColumnDataTypeHintEnum(column.TypeHint)), |
There was a problem hiding this comment.
These should be wrapped in pointer.ToEnum instead
|
|
||
| var defaultRetentionInDaysSentinelValue = pointer.To(int64(-1)) | ||
|
|
||
| type Column struct { |
There was a problem hiding this comment.
In case Column isn't unique later down the line, we should be more explicit and expand this out to something likeWorkspaceTableColumn
internal/services/loganalytics/log_analytics_workspace_table.go
Outdated
Show resolved
Hide resolved
| planAllowsCustomRetention := props.Plan != nil && *props.Plan == tables.TablePlanEnumAnalytics | ||
| if planAllowsCustomRetention { | ||
| if !pointer.From(props.RetentionInDaysAsDefault) { | ||
| state.RetentionInDays = pointer.From(props.RetentionInDays) | ||
| } | ||
| if !pointer.From(props.TotalRetentionInDaysAsDefault) { | ||
| state.TotalRetentionInDays = pointer.From(props.TotalRetentionInDays) | ||
| } |
There was a problem hiding this comment.
If this is O+C, do we even need to check if we want to set this value?
internal/services/loganalytics/log_analytics_workspace_table_custom_log_resource.go
Outdated
Show resolved
Hide resolved
| if metadata.ResourceData.HasChange("retention_in_days") { | ||
| props.RetentionInDays = defaultRetentionInDaysSentinelValue | ||
| if config.RetentionInDays != 0 { | ||
| props.RetentionInDays = pointer.To(config.RetentionInDays) | ||
| } | ||
| } | ||
|
|
||
| if metadata.ResourceData.HasChange("total_retention_in_days") { | ||
| props.TotalRetentionInDays = pointer.To(config.TotalRetentionInDays) | ||
| if config.TotalRetentionInDays == 0 { | ||
| props.TotalRetentionInDays = defaultRetentionInDaysSentinelValue | ||
| } | ||
| } |
There was a problem hiding this comment.
Is there a reason the logic here is flipped?
There was a problem hiding this comment.
Just an oversight, I'll make them the same.
There was a problem hiding this comment.
Should this should be moved up above the ---?
|
Also a test is failing |
Co-authored-by: Matthew Frahry <mbfrahry@gmail.com>
Co-authored-by: sreallymatt <106555974+sreallymatt@users.noreply.github.com>
c2dee3a to
dd79a33
Compare
|
Any timeframe for when this will be further reviewed ? |
mbfrahry
left a comment
There was a problem hiding this comment.
Hey @sreallymatt, this looks good but I did find one minor issue if you want to take a look
internal/services/loganalytics/log_analytics_workspace_table_custom_log_resource.go
Show resolved
Hide resolved
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |

Community Note
Description
Adds a new resource:
azurerm_log_analytics_workspace_table_custom_logPR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_resource- support for thething1property [GH-00000]This is a (please select all that apply):
Related Issue(s)
Fixes #0000
AI Assistance Disclosure
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the provider.
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
Note
If this PR changes meaningfully during the course of review please update the title and description as required.