Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion fwprovider/nodes/apt/datasource_standard_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ func (d *standardRepositoryDataSource) Read(
return
}

srp.importFromAPI(ctx, data)
ver := getProxmoxVersion(ctx, d.client)
srp.importFromAPI(ctx, data, ver)

resp.Diagnostics.Append(resp.State.Set(ctx, &srp)...)
}
Expand Down
30 changes: 25 additions & 5 deletions fwprovider/nodes/apt/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ import (
"github.com/hashicorp/terraform-plugin-framework/types"

customtypes "github.com/bpg/terraform-provider-proxmox/fwprovider/types/nodes/apt"
"github.com/hashicorp/terraform-plugin-log/tflog"

"github.com/bpg/terraform-provider-proxmox/proxmox"
api "github.com/bpg/terraform-provider-proxmox/proxmox/nodes/apt/repositories"
"github.com/bpg/terraform-provider-proxmox/proxmox/version"
)

// Note that most constants are exported to allow the usage in (acceptance) tests.
Expand Down Expand Up @@ -219,7 +223,7 @@ func (rp *modelRepo) importFromAPI(ctx context.Context, data *api.GetResponseDat
}

// importFromAPI imports the contents of an APT standard repository from the Proxmox VE API's response data.
func (srp *modelStandardRepo) importFromAPI(_ context.Context, data *api.GetResponseData) {
func (srp *modelStandardRepo) importFromAPI(_ context.Context, data *api.GetResponseData, proxmoxVersion *version.ProxmoxVersion) {
for _, repo := range data.StandardRepos {
if repo.Handle == srp.Handle.ValueString() {
srp.Description = types.StringPointerValue(repo.Description)
Expand All @@ -240,24 +244,24 @@ func (srp *modelStandardRepo) importFromAPI(_ context.Context, data *api.GetResp
}

// Set the index…
srp.setIndex(data)
srp.setIndex(data, proxmoxVersion)
// … and then the file path when the index is valid…
if !srp.Index.IsNull() {
// …by iterating through all repository files…
for _, repoFile := range data.Files {
// …and get the repository when the file path matches.
if srp.Handle.IsSupportedFilePath(repoFile.Path) {
if srp.Handle.IsSupportedFilePath(repoFile.Path, proxmoxVersion) {
srp.FilePath = types.StringValue(repoFile.Path)
}
}
}
}

// setIndex sets the index of the APT standard repository derived from the defining source list file.
func (srp *modelStandardRepo) setIndex(data *api.GetResponseData) {
func (srp *modelStandardRepo) setIndex(data *api.GetResponseData, proxmoxVersion *version.ProxmoxVersion) {
for _, file := range data.Files {
for idx, repo := range file.Repositories {
if slices.Contains(repo.Components, srp.Handle.ComponentName()) {
if slices.Contains(repo.Components, srp.Handle.ComponentName(proxmoxVersion)) {
// Return early for non-Ceph repositories…
if !srp.Handle.IsCephHandle() {
srp.Index = types.Int64Value(int64(idx))
Expand All @@ -279,3 +283,19 @@ func (srp *modelStandardRepo) setIndex(data *api.GetResponseData) {

srp.Index = types.Int64Null()
}

// getProxmoxVersion retrieves the Proxmox VE version from the API client, falling back to the minimum supported version
// if the API call fails.
func getProxmoxVersion(ctx context.Context, client proxmox.Client) *version.ProxmoxVersion {
ver := version.MinimumProxmoxVersion
if versionResp, err := client.Version().Version(ctx); err == nil {
ver = versionResp.Version
} else {
tflog.Warn(ctx, "Failed to determine Proxmox VE version, assuming minimum supported version.", map[string]any{
"error": err,
"assumed_version": ver.String(),
})
}

return &ver
}
25 changes: 12 additions & 13 deletions fwprovider/nodes/apt/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

"github.com/bpg/terraform-provider-proxmox/fwprovider/nodes/apt"
"github.com/bpg/terraform-provider-proxmox/fwprovider/test"
apitypes "github.com/bpg/terraform-provider-proxmox/proxmox/types/nodes/apt/repositories"
)

// Note that some "hard-coded" values must be used because of the way how the Proxmox VE API for APT repositories works.
Expand Down Expand Up @@ -51,7 +50,7 @@ func TestAccDataSourceRepo(t *testing.T) {
{
Config: te.RenderConfig(`
data "proxmox_virtual_environment_apt_repository" "test" {
file_path = "/etc/apt/sources.list"
file_path = "/etc/apt/sources.list.d/proxmox.sources"
index = 0
node = "{{.NodeName}}"
}`),
Expand All @@ -66,7 +65,7 @@ func TestAccDataSourceRepo(t *testing.T) {
resource.TestCheckResourceAttr(
"data.proxmox_virtual_environment_apt_repository.test",
apt.SchemaAttrNameTerraformID,
"apt_repository_"+strings.ToLower(te.NodeName)+"_etc_apt_sources_list_0",
"apt_repository_"+strings.ToLower(te.NodeName)+"_etc_apt_sources_list_d_proxmox_sources_0",
),
test.ResourceAttributesSet("data.proxmox_virtual_environment_apt_repository.test", []string{
"components.#",
Expand Down Expand Up @@ -180,7 +179,7 @@ func TestAccResourceRepoValidInput(t *testing.T) {
Config: te.RenderConfig(`
resource "proxmox_virtual_environment_apt_repository" "test" {
enabled = true
file_path = "/etc/apt/sources.list"
file_path = "/etc/apt/sources.list.d/debian.sources"
index = 0
node = "{{.NodeName}}"
}`),
Expand Down Expand Up @@ -213,11 +212,11 @@ func TestAccResourceRepoValidInput(t *testing.T) {
map[int]knownvalue.Check{
// The possible Debian version is based on the official table of the Proxmox VE FAQ page:
// - https://pve.proxmox.com/wiki/FAQ#faq-support-table
// - https://www.thomas-krenn.com/en/wiki/Proxmox_VE#Proxmox_VE_8.x
// - https://www.thomas-krenn.com/en/wiki/Proxmox_VE#Proxmox_VE_9.x
//
// The required Proxmox VE version for this provider is of course also taken into account:
// - https://github.com/bpg/terraform-provider-proxmox?tab=readme-ov-file#requirements
0: knownvalue.StringRegexp(regexp.MustCompile(`(bookworm)`)),
0: knownvalue.StringRegexp(regexp.MustCompile(`(trixie)`)),
},
),
),
Expand All @@ -226,7 +225,7 @@ func TestAccResourceRepoValidInput(t *testing.T) {
tfjsonpath.New(apt.SchemaAttrNameURIs),
knownvalue.ListPartial(
map[int]knownvalue.Check{
0: knownvalue.StringRegexp(regexp.MustCompile(`https?://ftp\.([a-z]+\.)?debian\.org/debian`)),
0: knownvalue.StringRegexp(regexp.MustCompile(`https?://([a-z]+\.)?debian\.org/debian/`)),
},
),
),
Expand All @@ -235,14 +234,14 @@ func TestAccResourceRepoValidInput(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
test.ResourceAttributes("proxmox_virtual_environment_apt_repository.test", map[string]string{
"enabled": strconv.FormatBool(true),
"file_path": "/etc/apt/sources.list",
"file_path": "/etc/apt/sources.list.d/debian.sources",
"index": strconv.FormatInt(0, 10),
"node": te.NodeName,
"id": fmt.Sprintf(
"apt_repository_%s_%s_%d",
strings.ToLower(te.NodeName),
apt.RepoIDCharReplaceRegEx.ReplaceAllString(
strings.TrimPrefix("/etc/apt/sources.list", "/"),
strings.TrimPrefix("/etc/apt/sources.list.d/debian.sources", "/"),
"_",
),
0,
Expand All @@ -259,7 +258,7 @@ func TestAccResourceRepoValidInput(t *testing.T) {
ImportStateId: fmt.Sprintf(
"%s,%s,%d",
strings.ToLower(te.NodeName),
apitypes.StandardRepoFilePathMain,
"/etc/apt/sources.list.d/debian.sources",
testAccResourceRepoIndex,
),
ImportStateVerify: true,
Expand All @@ -271,7 +270,7 @@ func TestAccResourceRepoValidInput(t *testing.T) {
Config: te.RenderConfig(`
resource "proxmox_virtual_environment_apt_repository" "test" {
enabled = false
file_path = "/etc/apt/sources.list"
file_path = "/etc/apt/sources.list.d/debian.sources"
index = 0
node = "{{.NodeName}}"
}`),
Expand Down Expand Up @@ -309,7 +308,7 @@ func TestAccResourceStandardRepoValidInput(t *testing.T) {
// Test the "Create" and "Read" implementations.
{
// PUT /api2/json/nodes/{node}/apt/repositories with handle = "no-subscription" will create a new
// entry in /etc/apt/sources.list on each call :/
// entry in /etc/apt/sources.list.d/proxmox.sources on each call :/
SkipFunc: func() (bool, error) {
return true, nil
},
Expand All @@ -321,7 +320,7 @@ func TestAccResourceStandardRepoValidInput(t *testing.T) {
// The provided attributes and computed attributes should be set.
Check: resource.ComposeTestCheckFunc(
test.ResourceAttributes("proxmox_virtual_environment_apt_standard_repository.test", map[string]string{
"file_path": "/etc/apt/sources.list",
"file_path": "/etc/apt/sources.list.d/proxmox.sources",
"handle": "no-subscription",
"node": te.NodeName,
"status": "1",
Expand Down
3 changes: 2 additions & 1 deletion fwprovider/nodes/apt/resource_standard_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ func (r *standardRepositoryResource) read(ctx context.Context, srp *modelStandar
}
}

srp.importFromAPI(ctx, data)
ver := getProxmoxVersion(ctx, r.client)
srp.importFromAPI(ctx, data, ver)

return true, nil
}
Expand Down
47 changes: 34 additions & 13 deletions fwprovider/types/nodes/apt/standard_repo_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/hashicorp/terraform-plugin-go/tftypes"

apitypes "github.com/bpg/terraform-provider-proxmox/proxmox/types/nodes/apt/repositories"
"github.com/bpg/terraform-provider-proxmox/proxmox/version"
)

// Ensure the implementations satisfy the required interfaces.
Expand Down Expand Up @@ -139,10 +140,10 @@ func (v StandardRepoHandleValue) CephVersionName() apitypes.CephVersionName {
}

// ComponentName returns the corresponding component name.
func (v StandardRepoHandleValue) ComponentName() string {
func (v StandardRepoHandleValue) ComponentName(proxmoxVersion *version.ProxmoxVersion) string {
if v.cvn == apitypes.CephVersionNameUnknown && v.kind != apitypes.StandardRepoHandleKindUnknown {
// For whatever reason the non-Ceph handle "test" kind does not use a dash in between the "pve" prefix.
if v.kind == apitypes.StandardRepoHandleKindTest {
// On PVE8, for whatever reason the non-Ceph handle "test" kind does not use a dash in between the "pve" prefix.
if proxmoxVersion != nil && !proxmoxVersion.SupportModernAptSources() && v.kind == apitypes.StandardRepoHandleKindTest {
return fmt.Sprintf("pve%s", v.kind)
}

Expand All @@ -169,17 +170,37 @@ func (v StandardRepoHandleValue) IsCephHandle() bool {
}

// IsSupportedFilePath returns whether the handle is supported for the given source list file path.
func (v StandardRepoHandleValue) IsSupportedFilePath(filePath string) bool {
switch filePath {
case apitypes.StandardRepoFilePathCeph:
return v.IsCephHandle()
case apitypes.StandardRepoFilePathEnterprise:
return !v.IsCephHandle() && v.kind == apitypes.StandardRepoHandleKindEnterprise
case apitypes.StandardRepoFilePathMain:
return !v.IsCephHandle() && v.kind != apitypes.StandardRepoHandleKindEnterprise
default:
return false
// The proxmoxVersion parameter is used to determine whether to use old (.list) or new (.sources) format paths.
// For versions below PVE 9: only old .list paths are supported.
// For PVE 9 and above: both old and new paths are supported for compatibility.
func (v StandardRepoHandleValue) IsSupportedFilePath(filePath string, proxmoxVersion *version.ProxmoxVersion) bool {
supportsModernPaths := proxmoxVersion == nil || proxmoxVersion.SupportModernAptSources()

if v.IsCephHandle() {
if supportsModernPaths {
return filePath == apitypes.StandardRepoFilePathCeph || filePath == apitypes.OldStandardRepoFilePathCeph
}

return filePath == apitypes.OldStandardRepoFilePathCeph
}

if v.kind == apitypes.StandardRepoHandleKindEnterprise {
if supportsModernPaths {
return filePath == apitypes.StandardRepoFilePathEnterprise || filePath == apitypes.OldStandardRepoFilePathEnterprise
}

return filePath == apitypes.OldStandardRepoFilePathEnterprise
}

if v.kind == apitypes.StandardRepoHandleKindNoSubscription || v.kind == apitypes.StandardRepoHandleKindTest {
if supportsModernPaths {
return filePath == apitypes.StandardRepoFilePathMain || filePath == apitypes.OldStandardRepoFilePathMain
}

return filePath == apitypes.OldStandardRepoFilePathMain
}

return false
}

// Type returns the type of the value.
Expand Down
Loading