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

Would like to have a "ignore_not_found" attribute on repository datasource #437

Closed
1 of 2 tasks
fscellos opened this issue Mar 29, 2024 · 6 comments
Closed
1 of 2 tasks
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@fscellos
Copy link
Contributor

fscellos commented Mar 29, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

In my use case i would like to apply a tf script that create repository if they don't exist in target nexus and import them if they already exist. Many types of repositories are managed through the tf script.

Repositories have to be created if some input var is set to true.
In first intention, what i do is the following

import {
  for_each = var.npm_active ? ["1"] : []
  id       = format("%s-npm", upper(var.trigramme))
  to       = nexus_repository_npm_hosted.npm[0]
}
resource "nexus_repository_npm_hosted" "npm" {
  count  = var.npm_active ? 1 : 0
  name   = format("%s-npm", upper(var.trigramme))
  online = true
  storage {
    blob_store_name                = upper(var.trigramme)
    strict_content_type_validation = true
    write_policy                   = "ALLOW"
  }
}

but it doesn't work as expected if the repository doesn't exist : "could not read repository".

Another solution was to use datasource to calculate a boolean if repository doesn't exist and only apply import stage if it exist (else, simply apply ressource).

But it doesn't work because the Get function always return error on get (error could be 404 or 500, error is always send back)

repo, err := client.Repository.Npm.Hosted.Get(resourceData.Id()) if err != nil { return err }

So i would like to add an optional property "ignore_not_found" in datasource, used by above code, that have effect to map 404 error to nil one in return and keep actual behaviour for other.

I'm ok to implement it if you're agree to take this issue into account.

New or Affected Resource(s)/Data Source(s)

At least maven, npm, nuget, pypi, raw but all can be modified

Pro feature

  • Yes. Pro license Required

Community Plugin

No

Potential Terraform Configuration

data "nexus_repository_npm_hosted" "npml"{
  name = "RepoName"
  ignore_not_found = true
}

References

No response

@fscellos fscellos added the enhancement New feature or request label Mar 29, 2024
@fscellos fscellos changed the title Would like to have a "ignore_not_found" attribute on repositoyr datasource Would like to have a "ignore_not_found" attribute on repository datasource Mar 29, 2024
@fscellos
Copy link
Contributor Author

fscellos commented Apr 8, 2024

Hello. Any Mainteners out there to discuss about this issue ? (i wouldn't like to begin dev on it if i'm not sure to merge it at the end)

@XREvo
Copy link

XREvo commented Apr 9, 2024

I tried to implement it today. Here is the branch on my fork: https://github.com/XREvo/terraform-provider-nexus/tree/allow-404-response

I added the flag ignore_not_found on each datasource and set up a simple mechanism to detect if API response error must/can be managed:

ignore_not_found, ok := resourceData.Get("ignore_not_found").(bool)
if !ok {
  ignore_not_found = false
}

repo, err := client.Repository.Conan.Proxy.Get(resourceData.Id())
if err != nil {
  if !(ignore_not_found && strings.Contains(err.Error(), "HTTP: 404")) {
    return err
  }
}

But the unit test that i added keep failing with:

--- FAIL: TestAccDataSourceRepositoryAptHostedNotFound (0.98s)
    testing_new_config.go:58: no "id" found in attributes
    testing_new.go:63: no "id" found in attributes
FAIL

This behavior is known on the Terraform's documentation to be met when you do not write a value in the "id" attribute of your state. And as far as I understand an empty string is not considered as a value, because if I inject a random string in the "id" attribute, the test pass.

FYI, the code used to read the data source use this code if we "bypass" the 404 error as previously stated:

if repo == nil {
  resourceData.SetId("")
  return nil
}

Unfortunately @fscellos I don't think we can implement it like you asked.

Maybe another approach would be to use a dedicated data source to find if a repository exists. I'm thinking of a data source like this one :

data "nexus_repository_apt_hosted_exists" "my-repo-exists" {
	name   = 'my-repo-name"
}

It would use the value of "name" as value for "id" and output a "exists" boolean flag to check if it exists or not.

What do you think of it?

@anmoel
Copy link
Member

anmoel commented Apr 10, 2024

hello @fscellos , hi @XREvo,

thank you for your input. i think it would be better to implement a new data source with name "nexus_repositories" that returns a list of all repositories. Your problem can be fixed like this:

locals {
  npm_repos = [
    "test1",
    "test2",
  ]
  import_npm_repos = { for repo in data.nexus_repositories.all.list : repo.name  => repo if !contains(local.npm_repos, repo.name) }
}

data "nexus_repositories" "all" {
}

import {
  for_each = local.import_npm_repos
  id       = each.value.name
  to       = nexus_repository_npm_hosted.npm[each.key]
}
resource "nexus_repository_npm_hosted" "npm" {
  for_each = local.npm_repos
  name   = each.key
  online = true
  storage {
    blob_store_name                = upper(var.trigramme)
    strict_content_type_validation = true
    write_policy                   = "ALLOW"
  }
}

with this solution it can be implemented with all types of repositories and in the future we can add search arguments to this data resource

@anmoel anmoel added good first issue Good for newcomers help wanted Extra attention is needed labels Apr 10, 2024
@fscellos
Copy link
Contributor Author

Hello @anmoel , @XREvo

As the nexus go-client used doesn't support yet search API i think using what you suggest (filtering datasources) is a good tradeoff.

Maybe, we can just add an optional "type" filter on this data source in order to control amount of data manipulated in terraform script (even if your example is ok for our use case) (this type data is send back in RepositoryInfo structure from nexus client).

As we already plan this feature in our internal board, i think we'll be able to submit you a PR in a couple of days.

@fscellos
Copy link
Contributor Author

fscellos commented Apr 19, 2024

Hello,

It appears that the suggested DataSource already exist in the provider (ie. "nexus_repository_list").
I just see it as i read the code.

But in my use case i also need to have such a feature for Blob stores, for which no datasource of type "list" exist.

It's not directly link to this issue but i push a PR to introduce, instead, a "nexus_blobstore_list". : PR 442
Fabrice.

@anmoel
Copy link
Member

anmoel commented Jun 13, 2024

Hi @fscellos,

your PR was released in version 2.3.0

@anmoel anmoel closed this as completed Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
Development

No branches or pull requests

3 participants