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

Added support for data_serviceendpoint_kubernetes #926

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

Conversation

muresan
Copy link

@muresan muresan commented Nov 10, 2023

All Submissions:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • My code follows the code style of this project.
  • I ran lint checks locally prior to submission.
  • Have you checked to ensure there aren't other open PRs for the same update/change?

I did not see tests added to the other data sources, so I did not add one for this one.

The linked issue has info about this PR

What about the current behavior has changed?

Issue Number: #877

Does this introduce a change to go.mod, go.sum or vendor/?

  • Yes
  • No

Does this introduce a breaking change?

  • Yes
  • No

Any relevant logs, error output, etc?

(If it’s long, please paste to https://ghostbin.com/ and insert the link here.)

Other information

@muresan
Copy link
Author

muresan commented Nov 10, 2023

@microsoft-github-policy-service agree

Copy link
Collaborator

@xuzhang3 xuzhang3 left a comment

Choose a reason for hiding this comment

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

@@muresan Thank you for your contribute. Can you help add AccTest to cover the new data source. And add separate schema(data source properties are simply compute=true) and flatten function for data source not simply reuse the resource functions.

@muresan
Copy link
Author

muresan commented Nov 19, 2023

@@muresan Thank you for your contribute. Can you help add AccTest to cover the new data source. And add separate schema(data source properties are simply compute=true) and flatten function for data source not simply reuse the resource functions.

Thank you for your comment. I added the kubernetes service endpoint in the same way as the sonar cloud service endpoint was added. Can you give me an example of another data source that I should follow ? All service endpoints data sources are added in the same way.
As for tests, can you point me to an example ? All the other serivice endpoint data souces had no tests so my assumption was that they are not desired. Can you point me to an example ?

Thanks.

@xuzhang3
Copy link
Collaborator

@muresan Data source only need the required parameter to get the service connection, other properties are not required. In the scenario, you are using the common schema for the k8s service connection. This is OK as the common schema cover the minimal properties requirement. You can add the same properties as azuredevops_serviceendpoint_kubernetes and all these properties are computed=true
Example:

r.Schema["service_account"] = &schema.Schema{
		Type:     schema.TypeList,
		MaxItems: 1,
		Computed: true,
		Elem: map[string]*schema.Schema{
			"ca_cert": {
				Type:     schema.TypeString,
				Computed: true,
			},
			"token": {
				Type:     schema.TypeString,
				Computed: true,
			},
		},
	}

You can ref: https://github.com/microsoft/terraform-provider-azuredevops/blob/main/azuredevops/internal/service/build/data_build_definition.go

@xuzhang3 xuzhang3 self-assigned this Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants