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
Alerting: Export of alert rules in HCL format #73166
Conversation
cf2abca
to
943f941
Compare
943f941
to
65ada50
Compare
65ada50
to
f0119e6
Compare
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.
LGTM from the FE side!🚀
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.
LGTM!
exportFormat: 'hcl', | ||
}; | ||
|
||
export const grafanaRuleExportProviders = { |
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.
🙌
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.
LGTM! Nice work!
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.
This accomplishes a lot with very little code added, awesome to see! Couple comments, mostly I'd love to hear how you feel about what would need to happen before this can be pulled out of alerting.
@@ -241,7 +241,7 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { | |||
disabled={submitState.loading} | |||
size="sm" | |||
> | |||
{isCortexLokiOrRecordingRule(watch) ? 'Edit YAML' : 'View YAML'} | |||
{isCortexLokiOrRecordingRule(watch) ? 'Edit YAML' : 'Export'} |
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.
This needs to be updated for the "View rule" page as well.
Tangentially, I feel like either this button or the title of the panel this opens need a new name.
RelativeTimeRange RelativeTimeRangeExport `json:"relativeTimeRange,omitempty" yaml:"relativeTimeRange,omitempty" hcl:"relative_time_range,block"` | ||
DatasourceUID string `json:"datasourceUid" yaml:"datasourceUid" hcl:"datasource_uid"` | ||
Model map[string]interface{} `json:"model" yaml:"model"` | ||
ModelString string `json:"-" yaml:"-" hcl:"model"` |
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.
It's unfortunate there's not a better way to format this without a lot more work 😞 Generating a Heredoc or plain HCL wrapped in jsonencode
would make the output a lot easier to read.
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.
It will be possible when\if hashicorp/hcl#608 is merged.
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestEncode(t *testing.T) { |
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.
Could this test be simpler? We don't necessarily need to worry about testing gohcl.EncodeAsBlock
or documenting how the field tags work, so we could reduce data
to a single field.
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 could not find a decent documentation about how the encoder works. So, I decided to write a test as a doc
@@ -517,3 +522,43 @@ func exportResponse(c *contextmodel.ReqContext, body definitions.AlertingFileExp | |||
} | |||
return r(http.StatusOK, body) | |||
} | |||
|
|||
func exportHcl(download bool, body definitions.AlertingFileExport) response.Response { |
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.
This is probably fine for a POC but I'm worried about how we'd be able to generalize this in the future.
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 think it will be possible when the approach to encoding a struct to different formats will use the same approach. I think if hashicorp/hcl#608 gets merged, we can unify this. For now, I think the separate function is more cleaner.
} | ||
|
||
// TODO implement support. | ||
// for idx, cp := range ex.ContactPoints { |
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.
Yuri, are you planning to do this TODO before merging this feature or will it come later?
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.
It will be addressed in a follow up PR
|
||
func Encode(resources ...Resource) (data []byte, err error) { | ||
defer func() { | ||
if r := recover(); r != nil { |
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 like this, we use it in the new Alertmanager label matchers parser too.
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.
yeah, Encoder is prone to panic :) when the struct has incorrect tags.
# Conflicts: # pkg/services/ngalert/api/tooling/definitions/provisioning_alert_rules.go # public/app/features/alerting/unified/api/alertRuleApi.ts # public/app/features/alerting/unified/components/export/providers.ts # public/app/features/alerting/unified/components/rule-editor/GrafanaRuleInspector.tsx
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.
LGTM, let's also remember to update the swagger definition when this is all finished. Maybe with // enum: yaml,json,hcl
in ExportQueryParams
* import hashicopr/hcl/v2 * add hcl package and export to HCL * annotate export structs --------- Co-authored-by: Konrad Lalik <konrad.lalik@grafana.com>
* import hashicopr/hcl/v2 * add hcl package and export to HCL * annotate export structs --------- Co-authored-by: Konrad Lalik <konrad.lalik@grafana.com>
What is this feature?
Adds export of alert rule groups in HCL format. It uses the
hashicorp/hcl/v2
module to encode a struct to HCL resource. The changes in the export models are because file provisioning and terraform models are slightly different.Demo
firefox_GUjTsGWL3K.mp4
Why do we need this feature?
To let users export resources in format that is acceptable by Grafana Terraform Provider.
Who is this feature for?
Those who use Terraform to provision alerting resources.
Which issue(s) does this PR fix?:
Related #72135
Special notes for your reviewer:
This PR implements only alert rules. Remaining resources will be addressed in the follow-up PRs.
Please check that: