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

Fixes for Katello repositories and sync_plans #143

Merged
merged 8 commits into from
Dec 18, 2023

Conversation

bitkeks
Copy link
Collaborator

@bitkeks bitkeks commented Dec 11, 2023

This PR collects fixes for Katello repositories and sync plans.

…g orgId

As seen in
https://apidocs.theforeman.org/katello/latest/apidoc/v2/products.html,
the organization_id is a required parameter for the GET endpoint. It is
now sent with the name query term.

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>
Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>
The "label" field was missing the "computed" attribute, marking it as
changed every time if not specified in the .tf file

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>
"mirror_on_sync" is deprecated and removed from Katello (in 4.9 it seems).
It is replaced by "mirroring_policy".

This commit also adds a JSON marshaler to katello_repository for better
handling of fields and use cases (especially the content_type).

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>
The "download_concurrency" parameter is not returned from the Katello
API, but still exists in the source code of Katello in v4.9. It's
unclear what happens with the value, if supplied. But as it is not
returned to reflect the Terraform input, TF will always see a change
from 0 -> X. Therefore, DiffSuppression captures theses cases.

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>
@bitkeks bitkeks self-assigned this Dec 11, 2023
@bitkeks bitkeks requested a review from lhw December 11, 2023 15:06
Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>
The "sync_date" field allows passing in a datetime as string. It
specifies the first time a sync plan should run. This field requires a
specific format to allow Go to parse the value.

Use the specified format: YYYY-MM-DD HH:MM:SS +0000, where '+0000' is
the timezone difference. A value of '+0000' means UTC.

Additionally, the time zone suffix was documented as "UTC". There is now
an exception handler for this case, converting "UTC" to "+0000"
internally.

The diff suppression func in this commit allows using shorter fields
like "2023-12-11", which is then parsed to "2023-12-11 00:00:00 +0000".

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>
Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>
@bitkeks bitkeks marked this pull request as ready for review December 12, 2023 09:35
@bitkeks bitkeks merged commit 875b6b7 into terraform-coop:master Dec 18, 2023
7 checks passed
bitkeks added a commit to mindfulsecurity/terraform-provider-foreman that referenced this pull request Dec 18, 2023
commit 875b6b7
Author: Dominik Pataky <33180520+bitkeks@users.noreply.github.com>
Date:   Mon Dec 18 08:20:04 2023 +0000

    Fixes for Katello repositories and sync_plans (terraform-coop#143)

    * (cherry pick) katello_product: fix product data source query by adding orgId

    As seen in
    https://apidocs.theforeman.org/katello/latest/apidoc/v2/products.html,
    the organization_id is a required parameter for the GET endpoint. It is
    now sent with the name query term.

    * katello_product: Fix orgID parameter in creation call

    * katello_repo: fix "label" field due to missing "computed"

    The "label" field was missing the "computed" attribute, marking it as
    changed every time if not specified in the .tf file

    * katello_repo: replace mirror_on_sync by mirroring_policy; marshalJSON

    "mirror_on_sync" is deprecated and removed from Katello (in 4.9 it seems).
    It is replaced by "mirroring_policy".

    This commit also adds a JSON marshaler to katello_repository for better
    handling of fields and use cases (especially the content_type).

    * katello_repo: Add DiffSuppression to "download_concurrency" parameter.

    The "download_concurrency" parameter is not returned from the Katello
    API, but still exists in the source code of Katello in v4.9. It's
    unclear what happens with the value, if supplied. But as it is not
    returned to reflect the Terraform input, TF will always see a change
    from 0 -> X. Therefore, DiffSuppression captures theses cases.

    * (cherry pick) Katello product: re-add SSL-related fields

    * katello_sync_plan: Handle "sync_date" diffs better

    The "sync_date" field allows passing in a datetime as string. It
    specifies the first time a sync plan should run. This field requires a
    specific format to allow Go to parse the value.

    Use the specified format: YYYY-MM-DD HH:MM:SS +0000, where '+0000' is
    the timezone difference. A value of '+0000' means UTC.

    Additionally, the time zone suffix was documented as "UTC". There is now
    an exception handler for this case, converting "UTC" to "+0000"
    internally.

    The diff suppression func in this commit allows using shorter fields
    like "2023-12-11", which is then parsed to "2023-12-11 00:00:00 +0000".

    * katello sync_plan: Add ValidateDiagFunc for sync_date value

    ---------

    Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>
bitkeks added a commit that referenced this pull request Dec 19, 2023
* Add job templates TF  resource (+data res) with API calls

Adds the Foreman "job templates" objects to the provider.
Use "foreman_jobtemplate" as a (data) resource to manage the templates
in Foreman.

Tested with Foreman v3.6

TODO: Handle inputs; example .tf file

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

* (snapshot) working on nested template inputs

Handling the nested template inputs in the new job templates is
difficult in the current state of the provider. In earlier
implementation, the compute profiles did the same, by utilising JSON
marshalling and unmarshalling. I'm trying new ways, so this commit is a
WIP snapshot.

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

* Implement and test create/destroy for job templates

After different approaches to handle the nested template input
resources/objecs, I switched back to JSON marshalling of the struct.
With this, the creation and destruction of job_template Terraform
resources now work.

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

* fix: handling of job template inputs (esp. id and template_id fields)

Uses the existing JSON unmarshaling logic from other structs to handle
the conversion of differnent value types.

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

* Clean up job templates (inputs), add example for job_template

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

* job_templates: Add import function

Tested with an existing job_template in Foreman, successfully imported.

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

* Fix data source job_template "name" search field

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

* job_templates: handle value_type error via omitempty, diffSuppress

Since the value_type value is not returned from the Foreman API, the
provider cannot read the value of existing template inputs. This commit
therefore introduces two things:

1. Use omitempty to skip the field when parsing to JSON, removing errors
   during updates (Foreman rejects the request because value_type cannot
   be an empty string)
2. Use a diff suppression function if value_type is read as empty
   string, but the default is "plain" and so the provider thinks there's
   an update to do.

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

* job_templates: Fix wrong order of template_inputs from API

The template inputs are a list inside the job_templates objects. After
creating the resources through the Foreman API, reading the job_template
might result in a different order of template_inputs. This causes
Terraform to think there was a change. This commit introduces a sort
function which keeps the template_inputs in order, according to their
unique ID.

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

* job_templates data source: fix read (use query)

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

* job_templates: Add test structure and tests for job_templates data source

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

* job_templates: Fix testdata and extend tests for resource

The testdata was missing two fields in the query response: template and
locked. These are not returned in the upstream Foreman API when the
query is used. They are only contained in the detailed GET request for a
specific resource at /api/job_templates/<ID>. This complicates the tests
and therefore the testdata now returns the fields as well, otherwise a
refactoring to split single_query and read would be needed.

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

* job_templates: Add remaining tests for job_template resource

Adds remaining test cases and functions for job_templates.

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

* Squashed commit of the following:

commit 875b6b7
Author: Dominik Pataky <33180520+bitkeks@users.noreply.github.com>
Date:   Mon Dec 18 08:20:04 2023 +0000

    Fixes for Katello repositories and sync_plans (#143)

    * (cherry pick) katello_product: fix product data source query by adding orgId

    As seen in
    https://apidocs.theforeman.org/katello/latest/apidoc/v2/products.html,
    the organization_id is a required parameter for the GET endpoint. It is
    now sent with the name query term.

    * katello_product: Fix orgID parameter in creation call

    * katello_repo: fix "label" field due to missing "computed"

    The "label" field was missing the "computed" attribute, marking it as
    changed every time if not specified in the .tf file

    * katello_repo: replace mirror_on_sync by mirroring_policy; marshalJSON

    "mirror_on_sync" is deprecated and removed from Katello (in 4.9 it seems).
    It is replaced by "mirroring_policy".

    This commit also adds a JSON marshaler to katello_repository for better
    handling of fields and use cases (especially the content_type).

    * katello_repo: Add DiffSuppression to "download_concurrency" parameter.

    The "download_concurrency" parameter is not returned from the Katello
    API, but still exists in the source code of Katello in v4.9. It's
    unclear what happens with the value, if supplied. But as it is not
    returned to reflect the Terraform input, TF will always see a change
    from 0 -> X. Therefore, DiffSuppression captures theses cases.

    * (cherry pick) Katello product: re-add SSL-related fields

    * katello_sync_plan: Handle "sync_date" diffs better

    The "sync_date" field allows passing in a datetime as string. It
    specifies the first time a sync plan should run. This field requires a
    specific format to allow Go to parse the value.

    Use the specified format: YYYY-MM-DD HH:MM:SS +0000, where '+0000' is
    the timezone difference. A value of '+0000' means UTC.

    Additionally, the time zone suffix was documented as "UTC". There is now
    an exception handler for this case, converting "UTC" to "+0000"
    internally.

    The diff suppression func in this commit allows using shorter fields
    like "2023-12-11", which is then parsed to "2023-12-11 00:00:00 +0000".

    * katello sync_plan: Add ValidateDiagFunc for sync_date value


Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

* job_templates: Remove new utils functions from branch

They are to be included in an extra branch, see discussion
#138 (comment)

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>

---------

Signed-off-by: Dominik Pataky <pataky@mindful-security.eu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants