Added support for _Root project parameters and updated README.md

[sundarok]

• Added new resource "resource_root_project" to support the parameters in _Root project level.
• Since the _Root project is already exists in TeamCity the current resource will not be used. Need a special resource that will add/update/delete only the _Root level configuration parameters. The same should be handled while destroying the resource also.
• Added tests for the Root project resource

[cvbarros]

@sundarok, thanks for your first PR 🎉 ! Your contribution is appreciated and very valuable!

This seems like a great progress in the the proposal lined up in #87
This was on my pending review/reply list, but the PR came before 😅
So, we can take the opportunity to have an open discussion about some points before proceeding:

1. I'm a bit skeptical about reusing the logic from resource_project, a different resource. While parts of the schema could be refactored and shared between them, the update logic coupling may not be clear to future contributors.

2. Root projects support a lot of configurations in common with projects - adding these features to the provider in resource_project will cause a duplication, and I can see a "pit of failure" pattern happening - it will be easy to add features to both places. In addition, tests and code will be bloated by replicating the features for both.

IMO, this feature is invaluable - The ability to configure settings at the _Root project which can be inherited to all projects should not be overlooked. At the same time, the implementation in the current form can open the points I've raised above.

Tomorrow I'll circle back to this and take a good look at the resource design and ponder over this a bit. I don't want to block feature implementers or contributors, but at the same time I feel responsible for steering the codebase along with the initial vision for the project.

Be back tomorrow! Cheers!

[cvbarros]

This was done this way to prevent OSX from adding headers that caused a lot of noise during unzipping in CI. Do you have an alternative? If not, we'll have to keep bsdtar in this case

[cvbarros]

Hi again 👋 @sundarok and @justinto-guidewire, after giving this a closer look, I've tried an approach that I'd like to run by you.

It involves adding a new field to the resource_project schema:

"root": {
	Type: schema.TypeBool,
	Optional: true,
	Default: false,
}

I've began by writing an acceptance test and tried to make it work.
Note that this test in particular doesn't have some checks such as CheckDestroy: testAccCheckTeamcityProjectDestroy

func TestAccTeamcityProject_Root(t *testing.T) {
	resName := "teamcity_project.root"
	var p api.Project

	resource.Test(t, resource.TestCase{
		PreCheck:     func() { testAccPreCheck(t) },
		Providers:    testAccProviders,
		Steps: []resource.TestStep{
			resource.TestStep{
				Config: testAccTeamcityProjectRoot,
				Check: resource.ComposeTestCheckFunc(
					testAccCheckTeamcityProjectExists(resName, &p),
					resource.TestCheckResourceAttr(resName, "name", "<Root project>"),
					resource.TestCheckResourceAttr(resName, "description", "Contains all other projects"),
					resource.TestCheckResourceAttr(resName, "root", "true"),
					resource.TestCheckResourceAttr(resName, "config_params.param1", "config_value1"),
					resource.TestCheckResourceAttr(resName, "config_params.param2", "config_value2"),
					resource.TestCheckResourceAttr(resName, "env_params.param3", "env_value1"),
					resource.TestCheckResourceAttr(resName, "env_params.param4", "env_value2"),
					resource.TestCheckResourceAttr(resName, "sys_params.param5", "sys_value1"),
					resource.TestCheckResourceAttr(resName, "sys_params.param6", "sys_value2"),
					testAccCheckProjectParameter(&p, api.ParameterTypes.Configuration, "param1", "config_value1"),
					testAccCheckProjectParameter(&p, api.ParameterTypes.Configuration, "param2", "config_value2"),
					testAccCheckProjectParameter(&p, api.ParameterTypes.EnvironmentVariable, "param3", "env_value1"),
					testAccCheckProjectParameter(&p, api.ParameterTypes.EnvironmentVariable, "param4", "env_value2"),
					testAccCheckProjectParameter(&p, api.ParameterTypes.System, "param5", "sys_value1"),
					testAccCheckProjectParameter(&p, api.ParameterTypes.System, "param6", "sys_value2"),
				),
			},
		},
	})
}
const testAccTeamcityProjectRoot = `
resource "teamcity_project" "root" {
  	root = true

	config_params = {
		param1 = "config_value1"
		param2 = "config_value2"
	}

	env_params = {
		param3 = "env_value1"
		param4 = "env_value2"
	}

	sys_params = {
		param5 = "sys_value1"
		param6 = "sys_value2"
	}
}
`

This would indicate to the config that this is a root project.
However, we have a conflict problem with name and description, fields that cannot be ever set for the Root project. Terraform doesn't have support for ConflictsWith to evaluate resource field values.
In order to circumvent that, I've looked for examples in other provider codebases and I believe CustomizeDiff can help in this case.

const (
	ResourceProjectRootId   = "_Root"
	ResourceProjectRootName = "<Root project>"
	ResourceProjectRootDescription = "Contains all other projects"
)
...
CustomizeDiff: func(diff *schema.ResourceDiff, i interface{}) error {
	root := diff.Get("root").(bool)
	if root {
		if v, ok := diff.GetOk("name"); ok && v.(string) != ResourceProjectRootName {
			return errors.New("'name' cannot be defined for the root project")
		}
		if v, ok := diff.GetOk("description"); ok && v.(string) != ResourceProjectRootDescription {
			return errors.New("'description' cannot be defined for the root project")
		}
	}
	// Validate required name if root = false
	if _, ok := diff.GetOk("name"); !ok && !root {
		return errors.New("'name' is required for non-root project")
	}

	return nil
},

Some other adjustments had to be made in order for all the tests to pass, like skipping destroy (but removing from the state with d.SetId("")), and fixing some panics regarding ParentProject.

I'd say this option could be highly beneficial avoiding the creation of another resource, and also doesn't add that much complexity. It struck me as intuitive and easy for the end user, too.

What do you say?

[justinto-guidewire]

Hi @cvbarros, we agree with your approach as well, could you provide the changes you mentioned in the above comments and we can add any missing tests or anything else, and then update this PR for you to review. Thanks!

[cvbarros]

👋 @justinto-guidewire I've pushed a branch with the idea: f6f4db8
However, I'd be way more happy if the contributions came from the original authors 👍

[cvbarros]

Hello @sundarok @justinto-guidewire , there hasn't been much activity here since we last spoke.
Is this something you'd still be willing to contribute/move forward?

Thanks!

[justinto-guidewire]

Hi @cvbarros , unfortunately, we have other priorities at the moment, and won't be contributing to add root project parameters anymore. Please feel free to close this PR, thanks!