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

Resource: azurerm_kubernetes_flux_configuration and azurerm_arc_kubernetes_flux_configuration - fix kustomization update, missing credentials. #24066

Conversation

jakubramut
Copy link
Contributor

Fix #23449

…bernetes_flux_configuration` - fix kustomization update, missing credentials.

Fix [hashicorp#23449](hashicorp#23449)
@jakubramut
Copy link
Contributor Author

hello @rcskosir,
It's my first PR in the repo, tried to follow all rules from the documentation.
If I did something wrong just let me know - will fix it.
I'd be very grateful if you could help me to move forward with my change.
The bug #23449 is a blocker for us.

@jakubramut
Copy link
Contributor Author

jakubramut commented Dec 1, 2023

Apologize for messaging you directly @ms-henglu and @stephybun.
However, I saw that you were responsible for last PR/comments related to service/kubernetes-cluster.
Could I ask you for a PR review?
It's my first PR, and I'm curious if it's good enough for you.
As we mentioned in the bug, it's a blocker for us, and we cannot update any kustomizations. When we forgot and run a kustomization update then only manual recreation helps.
I built the provider with changes from this PR included and then it works as expected.

Copy link
Member

@stephybun stephybun 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 this PR @jakubramut. Overall this looks fine, I think we can go one step further, since I believe someone could still hit this bug when updating continuous_reconciliation_enabled.

Could you please make the change suggested and redo your testing?

Comment on lines 680 to 688
if _, exists := metadata.ResourceData.GetOk("git_repository"); exists {
_, configurationProtectedSettings, err := expandGitRepositoryDefinitionModel(model.GitRepository)
if err != nil {
return err
}
properties.Properties.ConfigurationProtectedSettings = configurationProtectedSettings
} else if _, exists = metadata.ResourceData.GetOk("bucket"); exists {
_, properties.Properties.ConfigurationProtectedSettings = expandBucketDefinitionModel(model.Bucket)
}
Copy link
Member

Choose a reason for hiding this comment

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

This fixes the issue if kustomizations is updated, however if continuous_reconciliation_enabled were to be updated then users would hit the same bug since ConfigurationProtectedSettings was set to nil on line 644 and hasn't been repopulated.

I think we should pull this logic out and execute it after checking all the properties for changes and if ConfigurationProtectedSettings is still nil (so we don't unnecessarily process it again), so move this to line 695

if properties.Properties.ConfigurationProtectedSettings = nil {
// add logic here
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stephybun, thank you very much for your answer.
Just pushed changes according to what you proposed.
Everything has been tested - working as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stephybun, could you check my answer?
Please, it's urgent for us.

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Thanks @jakubramut LGTM 👍

@stephybun stephybun merged commit 344674f into hashicorp:main Jan 8, 2024
22 of 24 checks passed
@github-actions github-actions bot added this to the v3.87.0 milestone Jan 8, 2024
stephybun added a commit that referenced this pull request Jan 8, 2024
dduportal pushed a commit to jenkins-infra/azure that referenced this pull request Jan 17, 2024
<Actions>
<action
id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8">
        <h3>Bump Terraform `azurerm` provider version</h3>
<details
id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24">
            <summary>Update Terraform lock file</summary>
<p>&#34;hashicorp/azurerm&#34; updated from &#34;3.86.0&#34; to
&#34;3.87.0&#34; in file &#34;.terraform.lock.hcl&#34;</p>
            <details>
                <summary>3.87.0</summary>
<pre>Changelog retrieved
from:&#xA;&#x9;https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.87.0&#xA;ENHANCEMENTS:&#xA;&#xA;*
dependencies: updating to `v0.20240112.1095456` of
`github.com/hashicorp/go-azure-sdk` [GH-24477]&#xA;* dependencies:
updating to `v0.65.1` of `github.com/hashicorp/go-azure-helpers`
[GH-24479]&#xA;* `kusto`: updating to use the base layer from
`hashicorp/go-azure-sdk` rather than `Azure/go-autorest`
[GH-24477]&#xA;* `azurerm_container_group` - support for the `priority`
property [GH-24374]&#xA;* Data Source: `azurerm_application_gateway` -
support for the `trusted_client_certificate.data` property
[GH-24474]&#xA;&#xA;## 3.87.0 (January 11,
2024)&#xA;&#xA;FEATURES:&#xA;&#xA;* New Data Source:
`azurerm_network_manager`
([#24398](hashicorp/terraform-provider-azurerm#24398
New Resource:
`azurerm_security_center_server_vulnerability_assessments_setting`
([#24299](https://github.com/hashicorp/terraform-provider-azurerm/issues/24299))&#xA;&#xA;ENHANCEMENTS:&#xA;&#xA;*
dependencies: updating to `v0.20240111.1094251` of
`github.com/hashicorp/go-azure-sdk`
([#24463](hashicorp/terraform-provider-azurerm#24463
Data Source: `azurerm_mssql_database` - support for `identity`,
`transparent_data_encryption_enabled`,
`transparent_data_encryption_key_vault_key_id` and
`transparent_data_encryption_key_automatic_rotation_enabled`
([#24412](hashicorp/terraform-provider-azurerm#24412
Data Source: `azurerm_mssql_server` - support for
`transparent_data_encryption_key_vault_key_id`
([#24412](hashicorp/terraform-provider-azurerm#24412
`machinelearning`: updating to API Version `2023-10-01`
([#24416](hashicorp/terraform-provider-azurerm#24416
`paloaltonetworks`: updating to API Version `2023-09-01`
([#24290](hashicorp/terraform-provider-azurerm#24290
`azurerm_container_app` - update create time validations for
`ingress.0.traffic_weight`
([#24042](hashicorp/terraform-provider-azurerm#24042
`azurerm_container_app`- support for the `ip_security_restriction` block
([#23870](hashicorp/terraform-provider-azurerm#23870
`azurerm_kubernetes_cluster` - properties in
`default_node_pool.linux_os_config.sysctl_config` are now updateable via
node pool cycling
([#24397](hashicorp/terraform-provider-azurerm#24397
`azurerm_linux_web_app` - support the `VS2022` value for the
`remote_debugging_version` property
([#24407](hashicorp/terraform-provider-azurerm#24407
`azurerm_mssql_database` - support for `identity`,
`transparent_data_encryption_key_vault_key_id` and
`transparent_data_encryption_key_automatic_rotation_enabled`
([#24412](hashicorp/terraform-provider-azurerm#24412
`azurerm_postgres_flexible_server` - the `sku_name` property now
supports being set to `MO_Standard_E96ds_v5`
([#24367](hashicorp/terraform-provider-azurerm#24367
`azurerm_role_assignment` - support for the `principal_type` property
([#24271](hashicorp/terraform-provider-azurerm#24271
`azurerm_windows_web_app` - support the `VS2022` value for the
`remote_debugging_version` property
([#24407](hashicorp/terraform-provider-azurerm#24407
`azurerm_cdn_frontdoor_firewall_policy` - support for
`request_body_check_enabled` property
([#24406](https://github.com/hashicorp/terraform-provider-azurerm/issues/24406))&#xA;&#xA;BUG
FIXES:&#xA;&#xA;* Data Source: `azurerm_role_definition` - fix
`role_definition_id`
([#24418](hashicorp/terraform-provider-azurerm#24418
`azurerm_api_management` - the `sku_name` property can now be updated
([#24431](hashicorp/terraform-provider-azurerm#24431
`azurerm_arc_kubernetes_flux_configuration` - prevent a bug where
certain sensitive properties for `bucket` and `git_repository` were
being overwritten after an update to the resource is made
([#24066](hashicorp/terraform-provider-azurerm#24066
`azurerm_kubernetes_flux_configuration` - prevent a bug where certain
sensitive properties for `bucket` and `git_repository` were being
overwritten after an update to the resource is made
([#24066](hashicorp/terraform-provider-azurerm#24066
`azure_linux_web_app` - prevent a bug in App Service processing of
`application_stack` in updates to `site_config`
([#24424](hashicorp/terraform-provider-azurerm#24424
`azure_linux_web_app_slot` - Fix bug in App Service processing of
`application_stack` in updates to `site_config`
([#24424](hashicorp/terraform-provider-azurerm#24424
`azurerm_network_manager_deployment` - update creation wait logic to
better tolerate the api returning not found
([#24330](hashicorp/terraform-provider-azurerm#24330
`azurerm_virtual_machine_data_disk_attachment` - do not update
applications profile with disks
([#24145](hashicorp/terraform-provider-azurerm#24145
`azure_windows_web_app` - prevent a bug in App Service processing of
`application_stack` in updates to `site_config`
([#24424](hashicorp/terraform-provider-azurerm#24424
`azure_windows_web_app_slot` - prevent a bug in App Service processing
of `application_stack` in updates to `site_config`
([#24424](hashicorp/terraform-provider-azurerm#24424
`azurerm_maintenance_configuration` - set the `reboot` property in
flatten from `AlwaysReboot` to `Always`
([#24376](hashicorp/terraform-provider-azurerm#24376
`azurerm_container_app_environment` - the `workload_profile` property
can now be updated
([#24409](https://github.com/hashicorp/terraform-provider-azurerm/issues/24409))&#xA;&#xA;&#xA;</pre>
            </details>
        </details>
<a
href="https://infra.ci.jenkins.io/job/terraform-jobs/job/azure/job/main/1004/">Jenkins
pipeline link</a>
    </action>
</Actions>

---

<table>
  <tr>
    <td width="77">
<img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli
logo" width="50" height="50">
    </td>
    <td>
      <p>
Created automatically by <a
href="https://www.updatecli.io/">Updatecli</a>
      </p>
      <details><summary>Options:</summary>
        <br />
<p>Most of Updatecli configuration is done via <a
href="https://www.updatecli.io/docs/prologue/quick-start/">its
manifest(s)</a>.</p>
        <ul>
<li>If you close this pull request, Updatecli will automatically reopen
it, the next time it runs.</li>
<li>If you close this pull request and delete the base branch, Updatecli
will automatically recreate it, erasing all previous commits made.</li>
        </ul>
        <p>
Feel free to report any issues at <a
href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br
/>
If you find this tool useful, do not hesitate to star <a
href="https://github.com/updatecli/updatecli/stargazers">our GitHub
repository</a> as a sign of appreciation, and/or to tell us directly on
our <a
href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>!
        </p>
      </details>
    </td>
  </tr>
</table>

Co-authored-by: Jenkins Infra Bot (updatecli) <60776566+jenkins-infra-bot@users.noreply.github.com>
Copy link

github-actions bot commented May 1, 2024

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

azurerm_kubernetes_flux_configuration is removing secrets after adding kustomization blocks
2 participants