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.

Sign up for GitHub

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

Jump to bottom

Third times a charm for protected environments #938

Merged
merged 3 commits into from Mar 26, 2022
Merged

Third times a charm for protected environments #938

merged 3 commits into from Mar 26, 2022

Conversation

Shocktrooper
Copy link
Collaborator

Continuation of #750 on a new fork

github-actions[bot]
github-actions bot reviewed Mar 8, 2022
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome @Shocktrooper 👋

It looks like this is your first submission to the Terraform GitLab Provider! If you haven’t already done so, please make sure you have checked out our CONTRIBUTING.md guide to make sure your contribution has all the necessary elements in place for a successful approval.

Thanks again, and welcome to the community! 😃

@Shocktrooper
Copy link
Collaborator Author

@timofurrer here is the new PR on my fork. I left a comment in one of the files and not sure I put the right value. Other than that I am not sure if I modified the other files correctly to conform to the changes the provider went through

@armsnyder armsnyder marked this pull request as ready for review March 8, 2022 08:38
@armsnyder armsnyder marked this pull request as draft March 8, 2022 08:39
@Shocktrooper
Copy link
Collaborator Author

@timofurrer I have some unit tests failing and not sure why. If you have some free time to sync up I would like to try and figure out why they are failing. Go is not my strong suit

@timofurrer timofurrer mentioned this pull request Mar 10, 2022
Closed
@timofurrer timofurrer self-requested a review March 10, 2022 15:55
timofurrer
timofurrer requested changes Mar 10, 2022
View reviewed changes
Copy link
Member

@timofurrer timofurrer 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 taking that up! There are a few things which need to be addressed.

Also the examples are missing (see the examples/ folder).

internal/provider/resource_gitlab_project_environment.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_environment.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_environment.go Show resolved Hide resolved
internal/provider/resource_gitlab_project_environment.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_environment.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_protected_environment.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_protected_environment.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_protected_environment.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_protected_environment.go Show resolved Hide resolved
internal/provider/resource_gitlab_project_protected_environment.go Show resolved Hide resolved
@github-actions github-actions bot added the merge-conflict PR cannot be merged due to a merge conflict label Mar 11, 2022
@github-actions
Copy link

This pull request has merge conflicts. Please rebase your branch onto main.

@github-actions github-actions bot removed the merge-conflict PR cannot be merged due to a merge conflict label Mar 11, 2022
@github-actions
Copy link

Conflicts are resolved. Thank you! 😀

@timofurrer timofurrer linked an issue Mar 13, 2022 that may be closed by this pull request
Environments support #444
Closed
timofurrer
timofurrer requested changes Mar 16, 2022
View reviewed changes
internal/provider/resource_gitlab_project_environment.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_protected_environment.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_environment.go Show resolved Hide resolved
internal/provider/resource_gitlab_project_environment.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_environment.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_protected_environment_test.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_protected_environment_test.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_protected_environment_test.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_protected_environment_test.go Outdated Show resolved Hide resolved
internal/provider/resource_gitlab_project_protected_environment_test.go Outdated Show resolved Hide resolved
timofurrer added a commit to timofurrer/go-gitlab that referenced this pull request Mar 16, 2022
@timofurrer
environments: Implement missing created_at and updated_at fields
f0f40a3 
Required by:

* gitlabhq/terraform-provider-gitlab#938
@timofurrer timofurrer mentioned this pull request Mar 16, 2022
Merged
@Shocktrooper
Copy link
Collaborator Author

@armsnyder @timofurrer Not sure if there is a way to mark me permanently as a trusted contributor for this PR but for protected environments I do not have an EE license to test with and I have to do the functional tests for them. If you have a temp license or another way to do the tests I am all ears

@timofurrer
Copy link
Member

@Shocktrooper Unfortunately, we can't I'm afraid - Good news though is that you can obtain a free Ultimate license as a trial, from here: https://about.gitlab.com/free-trial/.

@Shocktrooper
Copy link
Collaborator Author

@timofurrer Perfect ty!

@timofurrer timofurrer marked this pull request as ready for review March 19, 2022 15:39
@timofurrer timofurrer added this to the v3.13.0 milestone Mar 20, 2022
@Shocktrooper
Copy link
Collaborator Author

@timofurrer I am also completely re-hauling the protected environments acceptance tests based on review comments on the project environments resource and other things. I should have it done later today

@timofurrer
Copy link
Member

timofurrer commented Mar 20, 2022 •
edited

@Shocktrooper sounds good! I've assigned it to the next release, which I plan to release shortly after GitLab 14.9 is out :)

I'm going to review it in the evening :)

Just re-request the review once you are done 🎉

@Shocktrooper
Copy link
Collaborator Author

Shocktrooper commented Mar 20, 2022 •
edited

@timofurrer Will do! Also want to get these 2 fields you added in assuming the go-gitlab work is done and released before 14.9? https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82980

@timofurrer
Copy link
Member

These two fields are already present in the responses of 14.8 - but just weren't documented. However, they are not yet in go-gitlab and I don't think they are of particular interest - at least not in this first iteration of these resources.

I'd ignore them for now :)

@Shocktrooper
Copy link
Collaborator Author

Shocktrooper commented Mar 20, 2022 •
edited

@timofurrer Sounds good! Also I have to go back to bed but I believe the acceptance tests are close to a done state pending any failures if you want to take a cursory check. I'll fix any review comments/failures in ~5 hours when I wake up.

@timofurrer
Copy link
Member

@Shocktrooper I'm actually working on a few shortcomings and fixing the tests. I'll keep you posted :)

@timofurrer
Copy link
Member

timofurrer commented Mar 20, 2022 •
edited

Alright @Shocktrooper I did couple of things here:

  • rebased to the latest main - make sure to pull it first if you want to change anything!
  • cleaned up Git history
  • fixed a couple of bugs (see my additional commit)
  • fixed the testing (see my additional commit)
  • adjusted the docs slightly

The tests should now pass and as far as I can tell the resource work fine :)

I'd say this is ready for a merge. Because I've changed quite some stuff I'd be happy if @armsnyder you can review :)

timofurrer
timofurrer previously approved these changes Mar 20, 2022
View reviewed changes
Copy link
Member

@timofurrer timofurrer left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@armsnyder WDYT ? 🏓

@Shocktrooper
Copy link
Collaborator Author

@timofurrer Want to resolve all of the pending threads as well?

@timofurrer
Copy link
Member

@Shocktrooper Yes, absolutely - I thought I did but obviously wasn't the case 🤦

@timofurrer timofurrer merged commit d18aee4 into gitlabhq:main Mar 26, 2022
@timofurrer timofurrer self-assigned this Mar 26, 2022
@github-actions
Copy link

This functionality has been released in v3.13.0 of the Terraform GitLab Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue. Thank you!

gravis pushed a commit to gravis/go-gitlab that referenced this pull request Sep 1, 2022
@timofurrer @gravis
environments: Implement missing created_at and updated_at fields
0c29271 
Required by:

* gitlabhq/terraform-provider-gitlab#938
@github-actions github-actions bot locked and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@github-actions github-actions[bot] github-actions left review comments

@timofurrer timofurrer timofurrer approved these changes

Assignees

@timofurrer timofurrer

Labels
documentation provider resource Adds or modifies a resource size/XL tests
Milestone
v3.13.0
3 participants
@Shocktrooper @timofurrer @armsnyder