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

Split ctfd_challenge in two separate resources #134

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

Conversation

pandatix
Copy link
Member

After #133, this PR would introduce breaking changes.
The reflection behind it comes from #131.

Inside this PR I drop ctfd_challenge in favor of 2 new resources:

  • ctfd_challenge_standard
  • ctfd_challenge_dynamic

Additionally to the features discussed in the issue (TL;DR better documentation, maintainability, and autocompletion), the internals changed toward a better fit of the CTFd API, and ease development of other providers for plugins.
For instance, when conceiving the Terraform provider for the ctfd-chall-manager plugin we would be able to reuse the ChallengeStandardResourceModel structure and ChallengeStandardResourceAttributes variable, in the way I implemented the ctfd_challenge_dynamic.

Reusing the ID for other resources (e.g. ctfd_file, ctfd_flag) is still possible.

Poke @Nishacid. I know you used it for GreHack24, your feedback on such an evolution would be welcome 😄

@pandatix pandatix added documentation Improvements or additions to documentation enhancement New feature or request go Pull requests that update Go code labels Dec 12, 2024
@pandatix pandatix requested a review from NicoFgrx December 12, 2024 10:53
@coveralls
Copy link

coveralls commented Dec 12, 2024

Pull Request Test Coverage Report for Build 12297700719

Details

  • 338 of 503 (67.2%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-2.7%) to 67.428%

Changes Missing Coverage Covered Lines Changed/Added Lines %
provider/challenge_common.go 9 15 60.0%
provider/challenge_standard_data_source.go 5 13 38.46%
provider/challenge_dynamic_data_source.go 83 121 68.6%
provider/challenge_dynamic_resource.go 207 320 64.69%
Totals Coverage Status
Change from base Build 12144207231: -2.7%
Covered Lines: 1594
Relevant Lines: 2364

💛 - Coveralls

Copy link
Member

@NicoFgrx NicoFgrx left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants