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

Add default project to organization default settings #1603

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

Conversation

netramali
Copy link
Contributor

@netramali netramali commented Feb 10, 2025

Description

In atlas, we can now update the default project hence we need to add default project id to the organization default settings.

Note:

  • I have added default project to the importer as well, is this a needed change?
  • Cleaning up resources for a default project is a little tricky because default project cannot be deleted so, while testing I needed to set the default project to be the original default project to be able to delete the resource.

Remember to:

External links

Include any links here that might be helpful for people reviewing your PR. If there are none, feel free to delete this section.

Output from Documentation Preview

Screenshot 2025-02-11 at 12 21 20 PM Screenshot 2025-02-24 at 1 09 25 PM

Output from acceptance tests

Please run applicable acceptance tests locally and include the output here. See testing.md to learn how to run acceptance tests.

If you are an external contributor, your contribution(s) will first be reviewed before running them against the project's CI pipeline.

Screenshot 2025-02-13 at 9 54 36 AM
Screenshot 2025-02-13 at 9 57 56 AM
Screenshot 2025-02-13 at 11 27 31 AM

@netramali netramali force-pushed the netramali/TF-14884-organizationd-default-project branch from 4779a2f to 369256f Compare February 11, 2025 17:37
@netramali netramali marked this pull request as ready for review February 13, 2025 17:41
@netramali netramali requested a review from a team as a code owner February 13, 2025 17:41
Copy link
Contributor

@ctrombley ctrombley Feb 13, 2025

Choose a reason for hiding this comment

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

Shouldn't we be setting values for default_agent_pool_id and default_project_id in the resourceTFEOrganizationDefaultSettingsRead method?

Something like:

	if organization.DefaultAgentPool != nil {
		d.Set("default_agent_pool_id", organization.DefaultAgentPool.ID)
	}

	if organization.DefaultProject != nil {
		d.Set("default_project_id", organization.DefaultProject.ID)
	}

When I was smoke testing I noticed that manual drift is not detected by the provider, and I think this is the reason why. I know that default_agent_pool_id is out of the scope of this PR, but if we include it here we will probably be patching a bug that hasn't been reported yet.

The manual drift scenario is probably worth including in an acceptance test as well.

Copy link
Contributor Author

@netramali netramali Feb 18, 2025

Choose a reason for hiding this comment

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

@ctrombley What do you mean by create a test for manual drift scenario?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ctrombley I tried testing this out and the states is out of sync with config, so you need to apply again. It leads to a non-empty apply.

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