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

feat: PC-14787 Enable specifying project in the command for importing #354

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

BSski
Copy link
Member

@BSski BSski commented Dec 5, 2024

Release Notes

It's now possible to pass resource's project in the terraform import command, e.g. terraform import nobl9_agent.agent project_name/object_name.

@BSski BSski changed the title PC-14787 Enable specifying project in the command for importing feat: PC-14787 Enable specifying project in the command for importing Dec 5, 2024
@n9-machine-user n9-machine-user added go Pull requests that update Go code enhancement New feature or request minor New functionality with at most minor breaking changes labels Dec 5, 2024
@BSski BSski force-pushed the PC-14787-enable-specifying-project-in-tf-import-command branch from 5b8ac3c to 3f56a22 Compare December 16, 2024 18:05
@BSski BSski marked this pull request as ready for review December 16, 2024 18:07
@BSski BSski force-pushed the PC-14787-enable-specifying-project-in-tf-import-command branch from 3b8ec5a to 6193c13 Compare December 16, 2024 18:08
@BSski BSski force-pushed the PC-14787-enable-specifying-project-in-tf-import-command branch from 4d4356e to 9ac41cb Compare December 16, 2024 18:18
labtom
labtom previously approved these changes Dec 17, 2024
Copy link
Contributor

@labtom labtom left a comment

Choose a reason for hiding this comment

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

2 minor comments, otherwise - LGTM.

docs/index.md Outdated Show resolved Hide resolved
templates/index.md.tmpl Outdated Show resolved Hide resolved
BSski and others added 5 commits December 17, 2024 11:14
Co-authored-by: Tomek Labuk <89924840+labtom@users.noreply.github.com>
Co-authored-by: Tomek Labuk <89924840+labtom@users.noreply.github.com>
…' of ssh://github.com/nobl9/terraform-provider-nobl9 into PC-14787-enable-specifying-project-in-tf-import-command
@@ -213,6 +213,17 @@ func resourceAgentDelete(ctx context.Context, d *schema.ResourceData, meta inter
return nil
}

func resourceAgentImport(_ context.Context, d *schema.ResourceData, _ interface{}) ([]*schema.ResourceData, error) {
project, resourceID := parseImportID(d.Id())
if project != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

what If project is empty? will it eventually use the default project? just asking

@@ -27,6 +27,8 @@ The Nobl9 Terraform Provider does not support the configuration of the following
- [SLO Annotations](https://docs.nobl9.com/features/slo-annotations/)
- [Alert Silence](https://docs.nobl9.com/alerting/alert-silence/)

The Nobl9 Terraform Provider supports `terraform import` command. For project-bound resources, use `project_name/service_name` format.
Copy link
Contributor

Choose a reason for hiding this comment

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

We tend to use project-scoped, I think I've never seen project-bound phrasing before.
In addition, it seems we're not supporting all objects in this iteration, for instance Alert Policy, we should inform users about that (you can opt to list them here or just mention that selected ones are supported).

Suggested change
The Nobl9 Terraform Provider supports `terraform import` command. For project-bound resources, use `project_name/service_name` format.
The Nobl9 Terraform Provider supports `terraform import` command for selected objects. For project-scoped resources, use `project_name/object_name` format, for non-project-scoped resources simply provide the object's name.

d.SetId(resourceID)
return []*schema.ResourceData{d}, nil
}

Copy link
Contributor

Choose a reason for hiding this comment

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

placeholder comment: why we're not adding acceptance tests for import?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request go Pull requests that update Go code minor New functionality with at most minor breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants