Skip to content

config-extractor: verify provider name against registered name in manager - #3235

Open
Amulyam24 wants to merge 3 commits into
confidential-containers:mainfrom
Amulyam24:verify-provider-name
Open

config-extractor: verify provider name against registered name in manager#3235
Amulyam24 wants to merge 3 commits into
confidential-containers:mainfrom
Amulyam24:verify-provider-name

Conversation

@Amulyam24

@Amulyam24 Amulyam24 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

The provider name in generated chart files was set from the directory name. It would be ideal to respect the registered name at runtime via AddCloudProvider() or fall back to the directory name.

@Amulyam24
Amulyam24 requested a review from a team as a code owner August 7, 2026 07:52
@Amulyam24 Amulyam24 changed the title config-extractor: verify registered provider name from manager.go config-extractor: verify provider name against register name in AddCloudProvider Aug 7, 2026
@Amulyam24 Amulyam24 changed the title config-extractor: verify provider name against register name in AddCloudProvider config-extractor: verify provider name against registered name in manager Aug 7, 2026

@stevenhorsman stevenhorsman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A bug that Bob has suggested and a possible optimisation

Comment thread src/cloud-providers/cmd/config-extractor/main.go
Comment thread src/cloud-providers/cmd/config-extractor/main.go Outdated
Comment thread src/cloud-providers/cmd/config-extractor/main.go Outdated
@Amulyam24
Amulyam24 force-pushed the verify-provider-name branch 5 times, most recently from 1c2080f to bb0917c Compare August 19, 2026 09:28
Comment thread src/cloud-providers/Makefile Outdated
Comment thread src/cloud-api-adaptor/ibmcloud-powervs/README.md Outdated
@Amulyam24
Amulyam24 force-pushed the verify-provider-name branch from bb0917c to 2d75c07 Compare August 19, 2026 14:36
Comment thread src/cloud-providers/cmd/config-extractor/main.go Outdated
@Amulyam24
Amulyam24 force-pushed the verify-provider-name branch 2 times, most recently from dd5b204 to 8b8e92d Compare August 20, 2026 04:55
@Amulyam24

Copy link
Copy Markdown
Contributor Author

Hey @stevenhorsman, can this PR be merged?

@stevenhorsman

Copy link
Copy Markdown
Member

Hey @stevenhorsman, can this PR be merged?

Not without second committer's review

exit 1
}

# Check for newly generated files that are not yet tracked by git

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So the expectation is to catch any mismatch with provider name and directory name ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, that correct,

Regenerate the provider yamls to align with the registered provider name - ibmcloud-powervs.

Signed-off-by: Amulyam24 <amulmek1@in.ibm.com>
The provider name in generated chart files was set from the directory
name rather than the name registered at runtime via AddCloudProvider().

Respect the registered name or fall back to the directory name if not found.

Signed-off-by: Amulyam24 <amulmek1@in.ibm.com>
Add a git ls-files --others --exclude-standard check after the
existing git diff to catch newly generated files that are not yet
tracked.

Signed-off-by: Amulyam24 <amulmek1@in.ibm.com>
@Amulyam24
Amulyam24 force-pushed the verify-provider-name branch from 8b8e92d to 03d9f19 Compare August 24, 2026 06:02
@Amulyam24
Amulyam24 requested a review from bpradipt August 24, 2026 08:04

@bpradipt bpradipt left a comment

Copy link
Copy Markdown
Member

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants