Skip to content

Conversation

@zjaco13
Copy link
Contributor

@zjaco13 zjaco13 commented Aug 30, 2024

Adds a null resource to cert manager to stop the timeout when creating

@elamaran11
Copy link
Contributor

@nimakaviani Please approve this. This is another bug we found while using reference-implementation.

Copy link
Contributor

@nabuskey nabuskey left a comment

Choose a reason for hiding this comment

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

Thanks for this! Just a minor comment to make sure we aren't using something that's deprecated.

})
}

resource "null_resource" "wait_for_cert_manager" {
Copy link
Contributor

Choose a reason for hiding this comment

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

null_resource is deprecated. let's use what's suggested in the doc here: https://registry.terraform.io/providers/hashicorp/null/latest/docs/resources/resource

Copy link
Contributor

@nabuskey nabuskey left a comment

Choose a reason for hiding this comment

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

Thanks for making the change. Just have a question about the new module being added. Please also fix DCO so we can merge: https://github.com/apps/dco

Comment on lines 21 to 24
http = {
source = "hashicorp/http"
version = ">= 3.4.4"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this module needed?

Signed-off-by: Zach Jacobson <[email protected]>
Copy link
Contributor

@nabuskey nabuskey left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you very much.

@nabuskey nabuskey merged commit bdfffed into cnoe-io:main Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants