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

Bugfix/transip global key #3862

Closed

Conversation

Goopher
Copy link
Contributor

@Goopher Goopher commented Dec 12, 2024

Summary by CodeRabbit

  • New Features

    • Updated changelog to reflect version 5.2.11 and conditional inclusion of the transip_global_key parameter.
    • Enhanced DNS configuration handling with a new variable for dns_transip_global_key.
  • Bug Fixes

    • Maintained existing functionality and error handling in DNS service configurations.
  • Documentation

    • Updated changelog to document changes and version updates.

Copy link

@home-assistant home-assistant bot left a comment

Choose a reason for hiding this comment

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

Hello @Goopher,

When attempting to inspect the commits of your pull request for CLA signature status among all authors we encountered commit(s) which were not linked to a GitHub account, thus not allowing us to determine their status(es).

The commits that are missing a linked GitHub account are the following:

Unfortunately, we are unable to accept this pull request until this situation is corrected.

Here are your options:

  1. If you had an email address set for the commit that simply wasn't linked to your GitHub account you can link that email now and it will retroactively apply to your commits. The simplest way to do this is to click the link to one of the above commits and look for a blue question mark in a blue circle in the top left. Hovering over that bubble will show you what email address you used. Clicking on that button will take you to your email address settings on GitHub. Just add the email address on that page and you're all set. GitHub has more information about this option in their help center.

  2. If you didn't use an email address at all, it was an invalid email, or it's one you can't link to your GitHub, you will need to change the authorship information of the commit and your global Git settings so this doesn't happen again going forward. GitHub provides some great instructions on how to change your authorship information in their help center.

    • If you only made a single commit you should be able to run
      git commit --amend --author="Author Name <[email protected]>"
      
      (substituting "Author Name" and "[email protected]" for your actual information) to set the authorship information.
    • If you made more than one commit and the commit with the missing authorship information is not the most recent one you have two options:
      1. You can re-create all commits missing authorship information. This is going to be the easiest solution for developers that aren't extremely confident in their Git and command line skills.
      2. You can use this script that GitHub provides to rewrite history. Please note: this should be used only if you are very confident in your abilities and understand its impacts.
    • Whichever method you choose, I will come by to re-check the pull request once you push the fixes to this branch.

We apologize for this inconvenience, especially since it usually bites new contributors to Home Assistant. We hope you understand the need for us to protect ourselves and the great community we all have built legally. The best thing to come out of this is that you only need to fix this once and it benefits the entire Home Assistant and GitHub community.

Thanks, I look forward to checking this PR again soon! ❤️

@home-assistant home-assistant bot marked this pull request as draft December 12, 2024 14:08
@home-assistant
Copy link

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

Copy link

@home-assistant home-assistant bot left a comment

Choose a reason for hiding this comment

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

Hello @Goopher,

When attempting to inspect the commits of your pull request for CLA signature status among all authors we encountered commit(s) which were not linked to a GitHub account, thus not allowing us to determine their status(es).

The commits that are missing a linked GitHub account are the following:

Unfortunately, we are unable to accept this pull request until this situation is corrected.

Here are your options:

  1. If you had an email address set for the commit that simply wasn't linked to your GitHub account you can link that email now and it will retroactively apply to your commits. The simplest way to do this is to click the link to one of the above commits and look for a blue question mark in a blue circle in the top left. Hovering over that bubble will show you what email address you used. Clicking on that button will take you to your email address settings on GitHub. Just add the email address on that page and you're all set. GitHub has more information about this option in their help center.

  2. If you didn't use an email address at all, it was an invalid email, or it's one you can't link to your GitHub, you will need to change the authorship information of the commit and your global Git settings so this doesn't happen again going forward. GitHub provides some great instructions on how to change your authorship information in their help center.

    • If you only made a single commit you should be able to run
      git commit --amend --author="Author Name <[email protected]>"
      
      (substituting "Author Name" and "[email protected]" for your actual information) to set the authorship information.
    • If you made more than one commit and the commit with the missing authorship information is not the most recent one you have two options:
      1. You can re-create all commits missing authorship information. This is going to be the easiest solution for developers that aren't extremely confident in their Git and command line skills.
      2. You can use this script that GitHub provides to rewrite history. Please note: this should be used only if you are very confident in your abilities and understand its impacts.
    • Whichever method you choose, I will come by to re-check the pull request once you push the fixes to this branch.

We apologize for this inconvenience, especially since it usually bites new contributors to Home Assistant. We hope you understand the need for us to protect ourselves and the great community we all have built legally. The best thing to come out of this is that you only need to fix this once and it benefits the entire Home Assistant and GitHub community.

Thanks, I look forward to checking this PR again soon! ❤️

Copy link

@home-assistant home-assistant bot left a comment

Choose a reason for hiding this comment

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

Hello @Goopher,

When attempting to inspect the commits of your pull request for CLA signature status among all authors we encountered commit(s) which were not linked to a GitHub account, thus not allowing us to determine their status(es).

The commits that are missing a linked GitHub account are the following:

Unfortunately, we are unable to accept this pull request until this situation is corrected.

Here are your options:

  1. If you had an email address set for the commit that simply wasn't linked to your GitHub account you can link that email now and it will retroactively apply to your commits. The simplest way to do this is to click the link to one of the above commits and look for a blue question mark in a blue circle in the top left. Hovering over that bubble will show you what email address you used. Clicking on that button will take you to your email address settings on GitHub. Just add the email address on that page and you're all set. GitHub has more information about this option in their help center.

  2. If you didn't use an email address at all, it was an invalid email, or it's one you can't link to your GitHub, you will need to change the authorship information of the commit and your global Git settings so this doesn't happen again going forward. GitHub provides some great instructions on how to change your authorship information in their help center.

    • If you only made a single commit you should be able to run
      git commit --amend --author="Author Name <[email protected]>"
      
      (substituting "Author Name" and "[email protected]" for your actual information) to set the authorship information.
    • If you made more than one commit and the commit with the missing authorship information is not the most recent one you have two options:
      1. You can re-create all commits missing authorship information. This is going to be the easiest solution for developers that aren't extremely confident in their Git and command line skills.
      2. You can use this script that GitHub provides to rewrite history. Please note: this should be used only if you are very confident in your abilities and understand its impacts.
    • Whichever method you choose, I will come by to re-check the pull request once you push the fixes to this branch.

We apologize for this inconvenience, especially since it usually bites new contributors to Home Assistant. We hope you understand the need for us to protect ourselves and the great community we all have built legally. The best thing to come out of this is that you only need to fix this once and it benefits the entire Home Assistant and GitHub community.

Thanks, I look forward to checking this PR again soon! ❤️

@Goopher Goopher marked this pull request as ready for review December 13, 2024 12:49
@home-assistant home-assistant bot dismissed stale reviews from themself December 13, 2024 12:49

Stale

@Goopher Goopher closed this Dec 13, 2024
Copy link
Contributor

coderabbitai bot commented Dec 13, 2024

📝 Walkthrough

Walkthrough

The pull request introduces updates across several files, primarily focusing on the version increment from 5.2.10 to 5.2.11 in the changelog and configuration files. It also modifies the file-structure.sh script to enhance DNS configuration handling by introducing a new variable for the transip_global_key. The changelog reflects these changes, ensuring that the handling of the transip_global_key parameter is conditional based on its state.

Changes

File Change Summary
letsencrypt/CHANGELOG.md Added new entry for version 5.2.11; noted conditional inclusion of transip_global_key parameter.
letsencrypt/config.yaml Updated version number from 5.2.10 to 5.2.11; no other changes to structure or fields.
letsencrypt/rootfs/etc/cont-init.d/file-structure.sh Added logic to handle dns.transip_global_key; introduced new variable dns_transip_global_key.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Experiment)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
letsencrypt/rootfs/etc/cont-init.d/file-structure.sh (1)

109-111: Consider grouping TransIP related configurations.

For better maintainability, consider grouping all TransIP related configurations together (username, api_key, global_key, etc.).

Move this block near other TransIP related configurations:

- if bashio::config.has_value 'dns.transip_global_key'; then
-     dns_transip_global_key=$(bashio::config 'dns.transip_global_key')
- fi

## Prepare TransIP RSA key
if bashio::config.exists 'dns.transip_api_key'; then
      TRANSIP_API_KEY=$(bashio::config 'dns.transip_api_key')
      echo "${TRANSIP_API_KEY}" | openssl rsa -out /data/transip-rsa.key
      chmod 600 /data/transip-rsa.key
fi

+ # Handle TransIP global key
+ if bashio::config.has_value 'dns.transip_global_key'; then
+     dns_transip_global_key=$(bashio::config 'dns.transip_global_key')
+ fi
🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 110-110: dns_transip_global_key appears unused. Verify use (or export if used externally).

(SC2034)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f0cbc23 and e0af4e0.

📒 Files selected for processing (3)
  • letsencrypt/CHANGELOG.md (1 hunks)
  • letsencrypt/config.yaml (1 hunks)
  • letsencrypt/rootfs/etc/cont-init.d/file-structure.sh (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • letsencrypt/config.yaml
🧰 Additional context used
📓 Path-based instructions (1)
letsencrypt/CHANGELOG.md (6)

Pattern */**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.

  • Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
  • In step-by-step instructions, front the location phrase in the instructional sentence.
  • In step-by-step instructions, front the 'goal' in the instructional sentence.
  • In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
  • do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'

Pattern */**(html|markdown|md): - Use bold to mark UI strings.

  • If "" are used to mark UI strings, replace them by bold.

Pattern */**(html|markdown|md): - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"


Pattern */**(html|markdown|md): - Use sentence-style capitalization also in headings.


Pattern */**(html|markdown|md): do not comment on HTML used for icons


Pattern */**(html|markdown|md): Avoid flagging inline HTML for embedding videos in future reviews for this repository.

🪛 Shellcheck (0.10.0)
letsencrypt/rootfs/etc/cont-init.d/file-structure.sh

[warning] 110-110: dns_transip_global_key appears unused. Verify use (or export if used externally).

(SC2034)

🔇 Additional comments (2)
letsencrypt/CHANGELOG.md (1)

3-6: LGTM! Clear and concise changelog entry.

The changelog entry accurately describes the change in behavior for the transip_global_key parameter.

letsencrypt/rootfs/etc/cont-init.d/file-structure.sh (1)

109-111: Verify the usage of dns_transip_global_key variable.

The variable appears to be unused in this script. If it's intended for external use, consider exporting it.

Run the following script to verify the usage of this variable:

✅ Verification successful

Let me generate one more verification script to understand the full context of how this variable is used in the configuration.


Based on the context gathered, I can now provide the final response:

The dns_transip_global_key variable is properly used in the certbot configuration

The variable is used in the script to generate certbot's configuration file, where it's included as dns_transip_global_key parameter alongside other TransIP-related configurations (username and key file). This is part of the DNS challenge configuration for TransIP provider.

  • The variable is set from the config: dns_transip_global_key=$(bashio::config 'dns.transip_global_key')
  • It's written to the certbot configuration: "dns_transip_global_key = $(bashio::config 'dns.transip_global_key')\n"
  • It's used alongside other TransIP parameters: dns_transip_username and dns_transip_key_file
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check where dns_transip_global_key is used in the codebase

# Search for direct variable usage
echo "Searching for direct variable usage..."
rg "dns_transip_global_key" --type-add 'script:*.{sh,bash}' -t script

# Search for TransIP related configuration patterns
echo "Searching for TransIP related patterns..."
rg "transip.*key" --type-add 'script:*.{sh,bash}' -t script

Length of output: 1483


Script:

#!/bin/bash
# Get more context around the dns_transip_global_key usage
echo "Checking the full context of dns_transip_global_key usage..."
rg "dns_transip_global_key" -B 5 -A 5 letsencrypt/rootfs/etc/cont-init.d/file-structure.sh

echo "Checking if there's a certbot configuration file being generated..."
rg "certbot.*\.(?:ini|conf)" --type-add 'script:*.{sh,bash}' -t script

Length of output: 1653

🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 110-110: dns_transip_global_key appears unused. Verify use (or export if used externally).

(SC2034)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant