Bug Fix: Initializing _certDataReader #9

joevanwanzeeleKF · 2025-10-29T15:04:41Z

bug fix: _certDataReader is now initialized in the Initialize method

… version of the integration

…sm for getting product ID's. Added check for a null response.

…ation.

…l values

Copilot

Pull request overview

This PR addresses a bug where the _certDataReader field was not being initialized in the Initialize method, along with multiple improvements to API response handling, logging, and spelling corrections throughout the codebase. The changes also update package dependencies and refactor how Vault API responses are deserialized by properly handling the wrapped response format.

Key changes:

Fixed bug where _certificateDataReader was not initialized in the Initialize method (line 57)
Changed RevocationTime from DateTime? to string in CertResponse to properly handle RFC3339 formatted timestamps from Vault API
Updated multiple package dependencies to newer versions, including several to version 10.0.0
Refactored Vault API calls to use WrappedResponse<T> wrapper for proper deserialization of nested response data
Corrected multiple spelling errors ("retreive" → "retrieve", "successfule" → "successful")

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
hashicorp-vault-cagateway/HashicorpVaultCAConnector.cs	Initializes `_certificateDataReader`, adds assembly version logging, changes RevocationTime handling from DateTime to string, fixes spelling errors, adds cancellation token to sync operations
hashicorp-vault-cagateway/Client/VaultHttp.cs	Enhanced logging, improved deserialization handling, fixed error message terminology
hashicorp-vault-cagateway/Client/HashicorpVaultClient.cs	Refactored to use WrappedResponse wrapper for API calls, fixed spelling in documentation
hashicorp-vault-cagateway/APIProxy/WrappedResponse.cs	Added RequestId and MountType properties to match Vault API response structure
hashicorp-vault-cagateway/APIProxy/CertResponse.cs	Changed RevocationTime from DateTime? to string, added RevocationTimestamp property
hashicorp-vault-cagateway/hashicorp-vault-caplugin.csproj	Updated package versions and added Microsoft.Extensions.Logging.Abstractions, removed Resources configuration
hashicorp-vault-cagateway/Properties/launchSettings.json	Removed launch settings file
readme_source.md	Fixed spelling error in documentation
CHANGELOG.md	Added entry documenting the bug fix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-17T22:13:48Z

hashicorp-vault-cagateway/HashicorpVaultCAConnector.cs

                            Certificate = certFromVault.Certificate,
                            Status = vaultStatus,
-                            RevocationDate = certFromVault.RevocationTime
+                            RevocationDate = !string.IsNullOrEmpty(certFromVault.RevocationTime) ? DateTime.Parse(certFromVault.RevocationTime) : null,                            


The DateTime.Parse call should specify culture and DateTimeStyles for consistent parsing. Line 173 properly handles this with CultureInfo.InvariantCulture and DateTimeStyles, but this line doesn't. Consider using the same pattern for consistency: DateTime.Parse(certFromVault.RevocationTime, System.Globalization.CultureInfo.InvariantCulture, System.Globalization.DateTimeStyles.AdjustToUniversal | System.Globalization.DateTimeStyles.AssumeUniversal)

Copilot · 2025-12-17T22:13:48Z

hashicorp-vault-cagateway/Client/VaultHttp.cs

-                    string serializedParams = JsonSerializer.Serialize(parameters, _serializerOptions);
-                    logger.LogTrace($"serialized parameters (from {parameters.GetType()?.Name}): {serializedParams}");
+                    string serializedParams = JsonSerializer.Serialize(parameters);
+                    logger.LogTrace($"deserialized parameters (from {parameters.GetType()?.Name}): {serializedParams}");


The log message refers to "deserialized parameters" but should be "serialized parameters" since the code is serializing the parameters object into JSON format.

Suggested change

logger.LogTrace($"deserialized parameters (from {parameters.GetType()?.Name}): {serializedParams}");

logger.LogTrace($"serialized parameters (from {parameters.GetType()?.Name}): {serializedParams}");

Copilot · 2025-12-17T22:13:49Z

hashicorp-vault-cagateway/Client/HashicorpVaultClient.cs

            {
                var res = await _vaultHttp.GetAsync<WrappedResponse<KeyedList>>("certs/?list=true");
-                return res.Data.Entries;
+                return res.Data?.Entries;


Potential null reference issue: The code accesses res.Data?.Entries without null-checking res itself. If the GetAsync call returns null, this will throw a NullReferenceException. Consider adding a null check for the response object before accessing its Data property.

Copilot · 2025-12-17T22:13:49Z

hashicorp-vault-cagateway/Client/HashicorpVaultClient.cs

-                var res = await _vaultHttp.GetAsync<KeyedList>("certs/revoked");
-                keys = res.Entries;
+                var res = await _vaultHttp.GetAsync<WrappedResponse<KeyedList>>("certs/revoked");
+                keys = res.Data?.Entries;


Potential null reference issue: The code accesses res.Data?.Entries without null-checking res itself. If the GetAsync call returns null, this will throw a NullReferenceException. Consider adding a null check for the response object before accessing its Data property.

Copilot · 2025-12-17T22:13:49Z

hashicorp-vault-cagateway/HashicorpVaultCAConnector.cs

-                        };
+                        };                    
+
+                        blockingBuffer.Add(newCert);


The blockingBuffer.Add() call at line 357 is missing the cancellation token parameter, while line 331 correctly includes it. For consistency and proper cancellation handling, this should be: blockingBuffer.Add(newCert, cancelToken);

Suggested change

blockingBuffer.Add(newCert);

blockingBuffer.Add(newCert, cancelToken);

Copilot · 2025-12-17T22:13:49Z

hashicorp-vault-cagateway/HashicorpVaultCAConnector.cs

-                        Status = certFromVault.RevocationTime != null ? (int)EndEntityStatus.REVOKED : (int)EndEntityStatus.GENERATED,
-                        RevocationDate = certFromVault.RevocationTime,                        
+                        Status = !string.IsNullOrEmpty(certFromVault.RevocationTime) ? (int)EndEntityStatus.REVOKED : (int)EndEntityStatus.GENERATED,
+                        RevocationDate = !string.IsNullOrEmpty(certFromVault.RevocationTime) ? DateTime.Parse(certFromVault.RevocationTime) : null


The DateTime.Parse call should specify culture and DateTimeStyles for consistent parsing. Line 173 properly handles this with CultureInfo.InvariantCulture and DateTimeStyles, but this line doesn't. Consider using the same pattern for consistency: DateTime.Parse(certFromVault.RevocationTime, System.Globalization.CultureInfo.InvariantCulture, System.Globalization.DateTimeStyles.AdjustToUniversal | System.Globalization.DateTimeStyles.AssumeUniversal)

Suggested change

RevocationDate = !string.IsNullOrEmpty(certFromVault.RevocationTime) ? DateTime.Parse(certFromVault.RevocationTime) : null

RevocationDate = !string.IsNullOrEmpty(certFromVault.RevocationTime)

? DateTime.Parse(

certFromVault.RevocationTime,

System.Globalization.CultureInfo.InvariantCulture,

System.Globalization.DateTimeStyles.AdjustToUniversal | System.Globalization.DateTimeStyles.AssumeUniversal)

: null

Copilot · 2025-12-17T22:13:50Z

hashicorp-vault-cagateway/HashicorpVaultCAConnector.cs

                    Certificate = cert.Certificate,
                    Status = revoked ? (int)EndEntityStatus.REVOKED : (int)EndEntityStatus.GENERATED,
-                    RevocationDate = cert.RevocationTime
+                    RevocationDate = cert.RevocationTime != null ? DateTime.Parse(cert.RevocationTime.ToString(), System.Globalization.CultureInfo.InvariantCulture, System.Globalization.DateTimeStyles.AdjustToUniversal | System.Globalization.DateTimeStyles.AssumeUniversal) : null


The RevocationDate parsing should handle the new string-based RevocationTime property more carefully. Since RevocationTime is now a string (changed in CertResponse.cs), you should check if it's null or empty before parsing: RevocationDate = !string.IsNullOrEmpty(cert.RevocationTime) ? DateTime.Parse(cert.RevocationTime) : null. The current code calls .ToString() on a string, which is redundant and may mask potential issues.

Copilot · 2025-12-17T22:13:50Z

hashicorp-vault-cagateway/HashicorpVaultCAConnector.cs

            string rawConfig = JsonSerializer.Serialize(configProvider.CAConnectionData);
-            logger.LogTrace($"serialized config: {rawConfig}");
            _caConfig = JsonSerializer.Deserialize<HashicorpVaultCAConfig>(rawConfig);
            logger.MethodExit(LogLevel.Trace);


The logger.MethodExit() call at line 55 is placed before the initialization completes. The method continues with additional initialization steps (lines 56-69) after this exit log. This should be moved to line 69, just before the closing brace, to accurately reflect when the method completes.

joevanwanzeeleKF added 10 commits October 29, 2025 11:01

assigning _certificateDataReader in initialize method

f38db6c

updated CHANGELOG

d730756

added additional logging in getCertificate

c9bc2bb

added additional logging for troubleshooting.

cf00704

added a tracelog in the initialize method that should log the current…

c6d7450

… version of the integration

Added missing fields for a wrapped response. Changed awaiting mechani…

7beec7d

…sm for getting product ID's. Added check for a null response.

swapped newtsonsoft json for system.text.json for consistent serializ…

b452a66

…ation.

logging full raw response, updated serialization config to ignore nul…

b650d91

…l values

added lookup table, extra logging, revocation check update

d605bb3

init and closing cert data reader

1f0ae06

spbsoluble requested a review from Copilot December 17, 2025 22:06

Copilot started reviewing on behalf of spbsoluble December 17, 2025 22:07 View session

Copilot AI reviewed Dec 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Bug Fix: Initializing _certDataReader #9

Bug Fix: Initializing _certDataReader #9

Uh oh!

joevanwanzeeleKF commented Oct 29, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	logger.LogTrace($"deserialized parameters (from {parameters.GetType()?.Name}): {serializedParams}");
	logger.LogTrace($"serialized parameters (from {parameters.GetType()?.Name}): {serializedParams}");

	blockingBuffer.Add(newCert);
	blockingBuffer.Add(newCert, cancelToken);

Bug Fix: Initializing _certDataReader #9

Are you sure you want to change the base?

Bug Fix: Initializing _certDataReader #9

Uh oh!

Conversation

joevanwanzeeleKF commented Oct 29, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants