Feat: support new vcs_connection resource #999
Conversation
| min_lower = 8 | ||
| } | ||
|
|
||
| # resource "env0_vcs_connection" "github" { |
There was a problem hiding this comment.
I can't run this test because I'm unsure if an agent (or what) is configured.
I will require assistance with this specific test. I am open to suggestions.
There was a problem hiding this comment.
we discussed in slack, TF-provider-integration-tests org has no agents, so don't test it for now (similar to how we don't test other agent operations such as project agent-assignment)
|
/review |
There was a problem hiding this comment.
I think this was a mistake... " copy..."
There was a problem hiding this comment.
Sorry I don't understand, what's the mistake?
There was a problem hiding this comment.
the file shouldn't be there... it got into the repo by mistake...
|
|
||
| type VcsConnectionUpdatePayload struct { | ||
| Name string `json:"name"` | ||
| VcsAgentKey string `json:"vcsAgentKey,omitempty"` |
There was a problem hiding this comment.
during update I don't think VcsAgentKey can be empty...
There was a problem hiding this comment.
so it's a PUT and not a PATCH?
There was a problem hiding this comment.
discussed internally. This field can be empty (null or undefined).
Therefore, omitempty is needed.
env0/resource_vcs_connection.go
Outdated
| "url": { | ||
| Type: schema.TypeString, | ||
| Required: true, | ||
| Description: "URL of the VCS server", |
There was a problem hiding this comment.
this can be either "VCS URL" (github.com) OR "Repository URL" (github.com/env0/myrepo)
There was a problem hiding this comment.
ok I will update the description.
There was a problem hiding this comment.
does it need to contain https:// as prefix?
There was a problem hiding this comment.
yes, either http or https (like I saw you already validate)
| func getVcsConnectionByName(name string, meta interface{}) (*client.VcsConnection, error) { | ||
| apiClient := meta.(client.ApiClientInterface) | ||
|
|
||
| vcsConnections, err := apiClient.VcsConnections() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| var foundConnections []client.VcsConnection | ||
|
|
||
| for _, connection := range vcsConnections { | ||
| if connection.Name == name { | ||
| foundConnections = append(foundConnections, connection) | ||
| } | ||
| } | ||
|
|
||
| if len(foundConnections) == 0 { | ||
| return nil, fmt.Errorf("VCS connection with name %v not found", name) | ||
| } |
There was a problem hiding this comment.
do we need to find Vcs-Connections by name? We don't have this in the BE
There was a problem hiding this comment.
if you mean adding a search query? I don't think so. this for loop should suffice.
(or I misunderstood?).
There was a problem hiding this comment.
I'm asking why do we need this entire function? Why do we need getVcsConnectionByName?
There was a problem hiding this comment.
oh! oh!
for import operation by name.
(imports can be by ID or Name).
| func TestNewStringInValidator(t *testing.T) { | ||
| allowedValues := []string{"one", "two", "three"} | ||
| validator := NewStringInValidator(allowedValues) |
There was a problem hiding this comment.
what are you testing here? related to vcs-connections?
There was a problem hiding this comment.
just added more unit tests...
the VCS is using some of these validators... (but not all of them).
| resource "env0_vcs_connection" "bitbucket_server" { | ||
| name = "bitbucket-server" | ||
| type = "BitBucketServer" | ||
| url = "https://bitbucket.example.com" |
There was a problem hiding this comment.
no vcs_agent_key on purpose? It may work with vcs_agent_key missing but better to be consistent
There was a problem hiding this comment.
yeah - needs to be fixed.
There was a problem hiding this comment.
so... follow-up question... do we want this field to be required ? right now it's optional in the API AFAIK.
There was a problem hiding this comment.
I suggest to keep it optional, like we describe in the API
| func (client *ApiClient) VcsConnectionUpdate(id string, payload VcsConnectionUpdatePayload) (*VcsConnection, error) { | ||
| var result VcsConnection | ||
|
|
||
| if err := client.http.Put("/vcs/connections/"+id, payload, &result); err != nil { |
There was a problem hiding this comment.
yeah - was just asking to be on the safe side...
github-actions
bot
added
ready to merge
Issue & Steps to Reproduce / Feature Request
resolves #994
Solution
Key Features: